Re: [Cellar] Lars Eggert's No Objection on draft-ietf-cellar-matroska-16: (with COMMENT)

[Steve Lhomme <[e-mail]>]

Hi Lars,

Thanks for you detailed review. Comments below.

> On 25 May 2023, at 09:08, Lars Eggert via Datatracker <[e-mail]> wrote:
> 
> Lars Eggert has entered the following ballot position for
> draft-ietf-cellar-matroska-16: No Objection
> 
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
> 
> 
> Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
> for more information about how to handle DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-cellar-matroska/
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> # GEN AD review of draft-ietf-cellar-matroska-16
> 
> CC @larseggert
> 
> Thanks to Elwyn Davies for the General Area Review Team (Gen-ART) review
> (https://mailarchive.ietf.org/arch/msg/gen-art/l2gsKdSbRBtCBNMrVZBmmRiu1IM).
> 
> ## Comments
> 
> ### Note to self
> 
> Check whether Expert Review is an appropriate registration policy here.
> 
> ### DOWNREFs
> 
> Possible DOWNREF from this Standards Track doc to `[Twofish]`. If so, the IESG
> needs to approve it.
> 
> Possible DOWNREF from this Standards Track doc to `[Blowfish]`. If so, the IESG
> needs to approve it.

I don’t have any particular opinion on this. If it is supported it is mandatory to know how.

> 
> ### Inclusive language
> 
> Found terminology that should be reviewed for inclusivity; see
> https://www.rfc-editor.org/part2/#inclusive_language for background and more
> guidance:
> 
> * Terms `mastertrackuid`, `masteringmetadata`, `mastering`,
>   `mastertrackseguid`, and `master`; alternatives might be `active`,
>   `central`, `initiator`, `leader`, `main`, `orchestrator`, `parent`,
>   `primary`, `server`

The “Master” element, is the term we already use in the EBML RFC 8794. We must use the same term here.

“Mastering” is also a technical term than would be confusing with another name.

> * Term `native`; alternatives might be `built-in`, `fundamental`, `ingrained`,
>   `intrinsic`, `original`
> * Term `blindly`; alternatives might be `visually impaired`, `unmindful of`,
>   `unconcerned about`, `negligent of`, `unaware`, `uncomprehending`,
>   `unaware`, `uncritical`, `unthinking`, `hasty`, `blocked`, `opaque`

These other instances have been fixed here: https://github.com/ietf-wg-cellar/matroska-specification/pull/788

> ## Nits
> 
> All comments below are about very minor potential issues that you may choose to
> address in some way - or ignore - as you see fit. Some were flagged by
> automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
> will likely be some false positives. There is no need to let me know what you
> did with these suggestions.
> 
> ### Duplicate references
> 
> Duplicate normative references to:
> `https://csrc.nist.gov/publications/detail/fips/197/final`.

Indeed, the SP.800-38A is using the wrong URL (bad copy past).

> 
> ### Outdated references
> 
> Document references `draft-ietf-cellar-tags-09`, but `-10` is the latest
> available revision.
> 
> Document references `draft-ietf-cellar-codec-09`, but `-10` is the latest
> available revision.

Fixed.

> 
> ### URLs
> 
> These URLs in the document can probably be converted to HTTPS:
> 
> * http://web.archive.org/web/20110214132246/http://labs.divx.com/node/16602
> * http://web.archive.org/web/20101222001148/http://labs.divx.com/node/16601
> * http://web.archive.org/web/20160609214806/https://www.fourcc.org/yuv.php
> * http://web.archive.org/web/20160609214806/https://www.fourcc.org/rgb.php
> 
Fixed.

> ### Grammar/style
> 
> #### Section 3, paragraph 3
> ```
> ch should have been used instead. Therefore Matroska writers MUST NOT use EBM
>                                  ^^^^^^^^^
> ```
> A comma may be missing after the conjunctive/linking adverb "Therefore".

Fixed all occurrences (start of a sentence).

> 
> #### Section 4.4, paragraph 33
> ```
> --------+ Figure 9: 	 a Attachments Element. The Tags Element
>                                      ^
> ```
> Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
> "an article", "an hour".

Fixed.
> 
> #### Section 5.1.4.1.29.2, paragraph 3
> ```
> s this is the ID of the public key the the data was encrypted with. stream c
>                                   ^^^^^^^
> ```
> Possible typo: you repeated a word.

Fixed all occurrences.

> 
> #### Section 5.1.4.1.31.6, paragraph 5
> ```
> g ID to identify the Chapter. For example it is used as the storage for [Web
>                                  ^^^^^^^
> ```
> A comma is probably missing here.

Fixed all occurrences.

> 
> #### Section 5.1.7.1.4.11, paragraph 1
> ```
> SHOULD include a CRC-32 Element as a their first Child Element. The Segment
>                                   ^^^^^^^
> ```
> A determiner cannot be combined with a possessive pronoun. Did you mean simply
> "a" or "their"?

Fixed
> 
> #### Section 5.1.8.1.1.3, paragraph 1
> ```
> yframe flag and Discardable flag. Otherwise everything is the same. Bit 0 is
>                                  ^^^^^^^^^
> ```
> A comma may be missing after the conjunctive/linking adverb "Otherwise".

Fixed all occurrences.
> 
> #### Section 6, paragraph 1
> ```
> er flags are set to 00. The Block for a 800 octets frame is as follows: +====
>                                      ^
> ```
> Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
> "an article", "an hour".

Fixed

> 
> #### Section 10.4.2, paragraph 8
> ```
> multiplied Block Timestamp is a floating values in nanoseconds. The Matroska
>                              ^^^^^^^^^^^^^^^^^
> ```
> The plural noun "values" cannot be used with the article "a". Did you mean "a
> floating value"?

Indeed. I used “a floating point value” in two places to fix this which is more precise.

> 
> #### Section 10.4.2, paragraph 8
> ```
> -2 form [ISO639-2] (like "fre" for french), or such a language code followed
>                                   ^^^^^^
> ```
> "French" needs to be capitalized when you are referring to the language or
> country.

I totally agree. Fixed.

> 
> #### Section 10.4.4, paragraph 1
> ```
> image SHOULD be presented with a 90 degree counter-clockwise rotation, with
>                                  ^^^^^^^^^
> ```
> When "90-degree" is used as a modifier, it is usually spelled with a hyphen.

Fixed.

> 
> #### Section 11.1.1, paragraph 3
> ```
> ding to the cumulative duration of the the previous Ordered Chapters. As an
>                                   ^^^^^^^
> ```
> Possible typo: you repeated a word.

Fixed.

> 
> #### Section 11.3, paragraph 2
> ```
> ack with a UID and all its flags. However the codec ID is meaningless becaus
>                                  ^^^^^^^
> ```
> A comma may be missing after the conjunctive/linking adverb "However".

Fixed all occurrences.

> 
> #### Section 12, paragraph 2
> ```
> red in the same channel as the track its linked to. When content is found in
>                                     ^^^
> ```
> Did you mean "it's" (contraction of "it is/has")?

Indeed. Fixed.

> 
> #### Section 16.2, paragraph 2
> ```
> during the film, which is an unusual specialized audio service that Matroska
>                             ^^^^^^^^^^^^^^^^^^^
> ```
> Make sure that the adjective "unusual" is correct. Possibly, it should be an
> adverb (typically ~ly) that modifies "specialized". Possibly, it should be the
> first word in a compound adjective (hyphenated adjective). Possibly, it is
> correct.

I think it’s correct here. But this is not my built-in language. I think it is for the original author of these lines.

> 
> #### Section 17, paragraph 1
> ```
> udio tracks and finds that multiple seem equally and maximally preferable, i
>                                    ^^^^
> ```
> The verb "seem" is plural. Did you mean: "seems"? Did you use a verb instead of
> a noun?

It think it’s a verb.

> 
> #### Section 17.2.1, paragraph 2
> ```
> only found in ordered chapters Furthermore there are other EBML Elements wh
>                                ^^^^^^^^^^^
> ```
> A comma may be missing after the conjunctive/linking adverb "Furthermore"

Fixed.

> #### Section 18.8, paragraph 2
> ```
> ChapterFlagHidden flag works independently from parent chapters. A Nested Ch
>                              ^^^^^^^^^^^^^^^^^^
> ```
> The usual collocation for "independently" is "of", not "from". Did you mean
> "independently of"?

Yes. Fixed.

> #### Section 19.2, paragraph 9
> ```
> d.(jpg|png). The filenames are case sensitive. The following table provides
>                               ^^^^^^^^^^^^^^
> ```
> This word is normally spelled with a hyphen.

Fixed,

> 
> #### Section 20.1.2, paragraph 1
> ```
> on. This Font Name can be different than the Attachment's FileName, even when
>                                    ^^^^
> ```
> Did you mean "different from"? "Different than" is often considered colloquial
> style.
> 
Yes. Fixed.

> #### Section 20.1.2, paragraph 6
> ```
> nts were used in existing files. Therefore it is RECOMMENDED for a Matroska
>                                 ^^^^^^^^^
> ```
> A comma may be missing after the conjunctive/linking adverb "Therefore".

Fixed,

> 
> #### Section 20.1.3, paragraph 1
> ```
> e file extension check MUST be case insensitive. Matroska writers SHOULD use
>                               ^^^^^^^^^^^^^^^^
> ```
> This word is normally spelled with a hyphen.

Fixed.
> 
> #### Section 20.2.2, paragraph 3
> ```
> e stream has neither a SeekHead list or a Cues list at the beginning of the
>                                     ^^
> ```
> Use "nor" with neither.

Fixed

> 
> #### Section 20.3, paragraph 2
> ```
> s Void Element should be adjusted depending whether the Matroska file already
>                                  ^^^^^^^^^
> ```
> The verb "depend" requires the preposition "on" (or "upon")

Fixed.

> #### Section 20.4, paragraph 2
> ```
> fore all the locations are known. Therefore shis layout is rarely used. * Se
>                                  ^^^^^^^^^
> ```
> A comma may be missing after the conjunctive/linking adverb "Therefore".
> Also, shis -> this?

Fixed both.

> 
> #### Section 20.4, paragraph 2
> ```
> a Reader could include: * Storage of a arbitrary and potentially executable d
>                                     ^
> ```
> Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
> "an article", "an hour".

Fixed.

> 
> #### Section 20.5.1, paragraph 3
> ```
> be allocated according to the "First Come First Served" policy [RFC8126]. The
>                                     ^^^^
> ```
> It seems that a comma is missing.

It is written multiple times without comma in RFC8126. It is the name of the rule, more than a sentence.

> 
> #### Section 27.1, paragraph 12
> ```
> be allocated according to the "First Come First Served" policy [RFC8126]. Cha
>                                     ^^^^
> ```
> It seems that a comma is missing.

Same as above.

> 
> #### Section 27.1, paragraph 12
> ```
> erTrackUID and MasterTrackSegUID should must be present and BlockGroups for t
>                                 ^^^^^^^^^^^
> ```
> Two modal verbs in a row. Did you mean: "should, must", "should" or "must"?

I think it’s “should” considering the next sentence.

You can find all the fixes mentioned above in https://github.com/ietf-wg-cellar/matroska-specification/pull/789

> 
> ## Notes
> 
> This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
> [`ietf-comments` tool][ICT] to automatically convert this review into
> individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].
> 
> [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
> [ICT]: https://github.com/mnot/ietf-comments
> [IRT]: https://github.com/larseggert/ietf-reviewtool
> 
> 
> 
> _______________________________________________
> Cellar mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/cellar