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 <slho...@matroska.org> 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 <nore...@ietf.org> >> 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 >> cel...@ietf.org <mailto:cel...@ietf.org> >> https://www.ietf.org/mailman/listinfo/cellar
_______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
