Re: [Cellar] Genart last call review of draft-ietf-cellar-matroska-15

[Steve Lhomme <[e-mail]>]

Hi all and especially Elwyn,

Most of the issues addressed here have been fixed in https://github.com/ietf-wg-cellar/matroska-specification/pull/735 and https://github.com/ietf-wg-cellar/matroska-specification/pull/736

And I added a renaming for “old parsers” in https://github.com/ietf-wg-cellar/matroska-specification/pull/809
And a clarification on the element attributes that are not written because they use their default/implied value: https://github.com/ietf-wg-cellar/matroska-specification/pull/810


> On 5 Mar 2023, at 15:15, Steve Lhomme <[e-mail]> wrote:
> 
> Hello,
> 
> First of all thanks for taking the time to read this lengthy document in detail :)
> 
>> On 3 Mar 2023, at 01:14, Elwyn Davies via Datatracker <[e-mail]> wrote:
>> 
>> Reviewer: Elwyn Davies
>> Review result: Almost Ready
>> 
>> I am the assigned Gen-ART reviewer for this draft. The General Area
>> Review Team (Gen-ART) reviews all IETF documents being processed
>> by the IESG for the IETF Chair.  Please treat these comments just
>> like any other last call comments.
>> 
>> For more information, please see the FAQ at
>> 
>> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
>> 
>> Document: draft-ietf-cellar-matroska-15
>> Reviewer: Elwyn Davies
>> Review Date: 2023-03-02
>> IETF LC End Date: 2023-02-28
>> IESG Telechat date: Not scheduled for a telechat
>> 
>> Summary:  Almost ready.  Thre is a good deal of discussion of earlir versions
>> of the structure and associated parsers together dicussion of future proofing
>> and potential future versions of the structure and associated parsers.  I am
>> concerned that it is not possible to automatically know which components are
>> associated with a given version.  At the least, this would assist implementers
>> to ensure that their parsers are working on the right ietms and ignoring
>> irrelevant items.
>> 
>> Major issues:
>> 
>> Versions of Matroska:  According to Section 2, this document covers versions 1,
>> 2, 3 and 4 of Matroska. The implication is that not all elements were defined
>> in lower numbered versions but that older parsers should potentially be able to
>> handle later versions of the format.  I am unclear about this but I have a
>> suspiscion that implementers and extenders of the format need to know which
>> elements existed in which versions and may need to understand whether they can
>> modify these elements in future versions.  At present there is no indication
>> which elements are defined in earlier versions and are therefore potentially
>> not updateable.
> 
> It is not written but it should be written in the specs. All elements have a “minver” value which is the minimum Matroska version they can be found in. But default the minver value 1 and not written in the document to minimise the length and make things more readable. Only elements with a higher minver value are specified as such. For example le SimpleBlock
> https://www.ietf.org/archive/id/draft-ietf-cellar-matroska-15.html#section-5.1.3.4
> 
> 
> This is using the minver attribute defined in RFC 8794, which has this definition:
> The minver attribute is OPTIONAL. If the minver attribute is not present, then the EBML Element has a minimum version of "1".
> 
> https://www.rfc-editor.org/rfc/rfc8794.html#name-minver
> 
> 
> The format is 20 years old and has a lot of widely used implementations. The core of Matroska and EBML is that it’s a binary format that can be extended, and has been in the past. This feature is more a EBML thing than a Matroska thing. And in the security considerations we did include the fact that readers must be prepared to handle elements they don’t know about:
> 
> Element IDs that are unknown to the EBML Reader MAY be accepted as valid EBML IDs in order to skip such elements.
> 
> https://www.rfc-editor.org/rfc/rfc8794.html#name-security-considerations
> 
>> Minor issues:
>> 
>> General: I am concerned about the long term stability of the web site
>> referenced for the Matroska Container Format, reference [MCF].  Among other
>> issues it is not accessed via https and it claims that it is the one true
>> specification which is rather confusing when it is being written into a
>> standards track RFC.
>> 
> 
> I lost the password to that page a long time ago but it has not moved since. I expect it to still be available for a long time as long a French ISP free.fr <http://free.fr/> is still running. Unfortunately I don’t think it will support HTTPS (custom sub domains for thousands of free hosting is probably not worth for the ISP).
> 
>> s1: What is meant by the term 'old parsers'?  Is this just claiming that
>> parsers for possible future formats will be always capable of parsing old
>> versions of the format?
> 
> Old parser = existing unmodified code (and the same for code from 15 to 20 years ago). This code should have been designed to handle any addition to the format. It may not have always been the case. For example a lot of parser don’t really support the Unknown-Size feature of EBML, but we have been constantly adding new elements without having to worry about breaking old code.
> 
>> 
>> Nits and Editorial Comments
>> 
>> Abstract and s1: I wonder if 'Matroska audiovisual data container structure'
>> might be a clearer reflection of what is being described?
>> 
> Yes, fixed by https://github.com/ietf-wg-cellar/matroska-specification/pull/735
> 
>> s1: It might be more helpful if the MCF reference pointed to the descriptive
>> introductory page of the web site (http://mukoli.free.fr/mcf/).
> 
> Indeed.
> 
>> s1, para 1: s/differentiates from it/diverges from it/
>> 
> OK
> 
>> s1, para 1: s/enables/provides/
> 
> OK
> 
>> s1, 2nd bullet: s/for which/in which/
> 
> OK
> 
>> s1, para after 2nd bullet: s/features like/features such as/
> 
> OK
> 
>> s4.3: I suspect that the use and format of Hexadecimal Floating-Point Constants
>> is not sufficiently generally understood to not require explanation in an RFC. 
>> I suggest duplicating the reference to [ISO9899] used in Section 11.1.18 of RFC
>> 8794 would be desirable.
> 
> I agree. I added a reference to ISO9899 in the text.
> 
>> 
>> s5: A reference to Section 11 of [RFC8794] referring to the structure of
>> element definitions would be useful.
> 
> OK, I’m adding a text link to section 11.1 which is the one that defines the actual EBML Schema elements and attributes.
> 
>> 
>> s5.1: The details of the elements in this section are outside my competence and
>> I haven't looked at them with any exactitude.  Nothing jumped out at me.
>> 
>> s6.1, para 2: I was unable to parse the second sentence: "In that case the
>> Segment containing in these Chapters do no required a Track Element or
>> a Cluster Element."
> 
> Rephrased to
> A Segment containing these linked Chapters does not require a Track Element or a Cluster Element.
> 
>> s20.5.2: s/contain/contains/
> 
> OK
> 
>> s23.3.3, para 1: s/want to seek/wants to seek/; s/to have these/to access these/
> 
> I used a plural for “players” as “they” was used later in the sentence.
> 
>> s27.1: Should an Element ID registry entry contain the Matroska version at
>> which it was introduced?
> 
> I would say no. The registry is mostly used to avoid collision. The optional link to the description should give enough information to use the element. If this is not the case, people will just not use this element and skip it when it’s found.
> 
>> s28, para 2: s/if there is no more/if there are no more/
> 
> OK.
> 
> You can find of the changes in this Pull Request https://github.com/ietf-wg-cellar/matroska-specification/pull/736
> 
> Thanks again for your help.
> 
> Steve
> 
>> 
>> 
>> 
>> 
>> _______________________________________________
>> Cellar mailing list
>> [e-mail] <mailto:[e-mail]>
>> https://www.ietf.org/mailman/listinfo/cellar