Re: [Cellar] Second AD review of draft-ietf-cellar-ebml-10

["Alexey Melnikov" <[e-mail]>]

Hi Dave,
I removed comments where we are in agreement. A few followups:

On Sat, Jul 6, 2019, at 6:23 PM, Dave Rice wrote:
> Hello Alexey,
> 
> Thanks much for providing this thorough review. I’ll try to reply point by point below with references to either pull requests or issues or offer followup questions. In a few places I ping Steve Lhomme and Michael Richardson as the comments relate to text they originated in their work.
> 
>> EBMLElementOccurrence = [EBMLMinOccurrence] "*" [EBMLMaxOccurrence]
>> EBMLMinOccurrence = 1*DIGIT
>> EBMLMaxOccurrence = 1*DIGIT
>> 
>> Are there any upper limits on allowed values for these fields? Even if you don't encode them using ABNF, it would be good to mention them in an ABNF comment.
>> 
>> VariableParentOccurrence = [PathMinOccurrence] "*" [PathMaxOccurrence]
>> PathMinOccurrence = 1*DIGIT
>> PathMaxOccurrence = 1*DIGIT
>> 
>> Same comment as above.
> 
> I looked for examples of that sort of commenting but didn’t find much guidance. Eventually I simply appended " ; no upper limit” to each of the four referenced lines and added that to https://github.com/cellar-wg/ebml-specification/pull/265.
I think this is the wrong fix. Is it sufficient for an implementation to use 32 bit value to represent any of these? 64 bit value?
"no upper limit" is not going to be interoperable.

> 
>> 11.1.10.1. label
>> 
>>  The label provides a concise expression for human consumption that
>>  describes what the value of the "<enum>" represents.
>> 
>> Is it worth adding a cross reference to the "lang" attribute here?
> 
> Do you mean to express the language of the term used within the label? Currently the language of the label is undefined and since it is an attribute that label is not repeatable.

To be honest I am not yet sure how I feel about "undefined language" here. Need to think about that.
But either way, I think adding some text that "lang" attribute doesn't apply would be helpful.

> 
>> 12. Considerations for Reading EBML Data
>> 
>>  If a Master Element contains a CRC-32 Element that doesn't validate,
>>  then the EBML Reader MAY ignore all contained data except for
>>  Descendant Elements that contain their own valid CRC-32 Element.
>> 
>> I don't fully understand your use of "MAY ... except ..." here.
>> Can you elaborate on why would an implementation ignore data contained in a Master Element and not ignore Descendant Elements, even if they own CRC-32 is valid?
> 
> For instance if a Matroska file has three metadata tags and each has a CRC value and so does the parent Tags element like this.
> 
> <Tags crc=invalid>
>  <Tag crc=valid>
>  <Tag crc=valid>
>  <Tag crc=invalid>
> <Tags>
> 
> We’re trying to say that even though the contents of the <Tags> element is invalid, that the valid child elements may still be used.
So to me this means that after discarding all invalid elements you end up with something like this:

<Tags >
 <Tag crc=valid>
</Tags>

As this is an incomplete document, I am struggling to understand what it can be used for?

>  An invalid descendant element would make all ancestor element invalid, but should not necessary make all sibling elements unusable.
> 

Best Regards,
Alexey