Hi all and especially Elwyn,
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 <slhomme@xxxxxxxxxxxx> 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 <noreply@xxxxxxxx> 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 SimpleBlockhttps://www.ietf.org/archive/id/draft-ietf-cellar-matroska-15.html#section-5.1.3.4This 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-minverThe 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-considerationsMinor 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 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/735s1: 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/
OKs1, para 1: s/enables/provides/
OKs1, 2nd bullet: s/for which/in which/
OKs1, para after 2nd bullet: s/features like/features such as/
OKs4.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 toA Segment containing these linked Chapters does not require a Track Element or a Cluster Element.s20.5.2: s/contain/contains/
OKs23.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/736Thanks again for your help.Steve
_______________________________________________
Cellar mailing list
Cellar@xxxxxxxx
https://www.ietf.org/mailman/listinfo/cellar
|
--
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call
