Re: [Cellar] Publication has been requested for draft-ietf-cellar-flac-08

[Martijn van Beurden <[e-mail]>]

Hi all,

With some help, I've tried to address all the raised issues to the best of
my abilities. I have uploaded a new I-D, draft-ietf-cellar-flac-10. The
diff is quite long, because I've used a grammar checker as suggested, which
found a lot of possible improvements.

Kind regards, Martijn van Beurden

Op ma 31 jul 2023 om 09:43 schreef Martijn van Beurden <[e-mail]>:

> Hi all,
>
> Currently I'm not sure how to proceed on updating the draft. Could someone
> please help with some of the questions in the email below?
>
> Kind regards, Martijn van Beurden
>
> Op ma 10 jul 2023 om 10:00 schreef Martijn van Beurden <[e-mail]>:
>
>>
>>
>> Op ma 10 jul 2023 om 04:19 schreef Murray S. Kucherawy <
>> [e-mail]>:
>>
>>> On Tue, Apr 4, 2023 at 9:12 PM Spencer Dawkins via Datatracker <
>>> [e-mail]> wrote:
>>>
>>>> Spencer Dawkins has requested publication of draft-ietf-cellar-flac-08
>>>> as Proposed Standard on behalf of the CELLAR working group.
>>>>
>>>> Please verify the document's state at
>>>> https://datatracker.ietf.org/doc/draft-ietf-cellar-flac/
>>>>
>>>
>>> Sorry for the long delay getting to this review.  I have to admit, as I
>>> think I did for Matroska, that codecs are not an area of my strength.  I've
>>> focused primarily here on the document's overall readiness to move forward
>>> in the process, but could easily have missed something specific to the way
>>> codecs work.  That said, it was an interesting read and I learned from it.
>>>
>>
>> Many thanks for your review. I have a few questions about some of the
>> raised issues,
>>
>>
>>>
>>> Here are my comments, in no particular order:
>>>
>>> In Section 13.1, the parameters say "None" but they should say "N/A";
>>> see Section 5.6 of RFC 6838.
>>>
>>>
>> OK, I'll change that.
>>
>>
>>> Also related to the media type registration, RFC 6838 requires that
>>> Security Considerations of the registration include, or refer to, an
>>> explicit statement about whether the payload found in the media type object
>>> is executable code.  I think this can be resolved by just adding a sentence
>>> to Section 12 of this document that makes that explicit one way or the
>>> other.
>>>
>>>
>> Okay, I'll add that back in. It used to say FLAC carries no executable
>> content, but there is no reason to state that. In fact, I know some people
>> use FLAC to store mixed-mode CD images, which do contain executable code.
>> So perhaps I'll state that FLAC files may contain executable code, although
>> the FLAC format is not designed for it and it is uncommon.
>>
>>
>>>
>>> I think IEC.60908.1999 should be a normative reference.  I think the
>>> references to RFC 6838 and RFC 7942 should be normative, but I also don't
>>> think they are necessary and could be removed to simplify things a bit.
>>>
>>>
>> Please explain. I thought normative references specify documents that
>> must be read to understand or implement this document? How can the
>> reference be a required read only when present?
>>
>> As for IEC.60908.1999, could you please explain what parts of the
>> specification are hard to understand without having read it? I don't have
>> access to IEC.60908.1999, I have never read it myself, and as far as I know
>> all relevant parts have already been explained in this document. From my
>> point of view, the reference is only there to point to the origin of some
>> fields.
>>
>>
>>>
>>> In Section 12, there's a reference to a github link.  I'm not sure how
>>> this will be received by the RFC Editor or the IESG.  What guarantees are
>>> there that this link is permanent?  This goes for other github links, e.g.,
>>> in Section 10.3.
>>>
>>>
>> Considering the other github links, the working group considered github a
>> more stable resource than IETF's trac. If I recall correctly the IETF wiki
>> wasn't ready when these parts were prepared. Do you think we should move
>> these resources to the IETF wiki?
>>
>> Considering the github link in section 12 (which I don't think we can
>> move to the wiki), the working group considered these permanent. Does the
>> IETF provide for storing a collection of large files somewhere permanent,
>> so these files can be transferred there? Perhaps the link should go to an
>> IETF wiki page that points to the github so the link can be updated if
>> necessary?
>>
>>
>>>
>>> There are numerous in-document references (i.e., to other sections) that
>>> render as links to github where this document is developed.  Is this
>>> generated by the xml2rfc code, or something else?  They will need to become
>>> regular section references before this moves ahead.  This may just be my
>>> unfamiliarity with recent changes in the xml2rfc tooling, but it stood out
>>> to me.
>>>
>>>
>> Can you name an example? I just checked and as far as I can see, there
>> are a few links to the github wiki (which can be moved to the IETF wiki if
>> necessary), one link to the github repo for the example files, the github
>> repo which test files and one to the FLAC source code repo for the full MP4
>> mapping.
>>
>>
>>>
>>> Regarding the SHOULD in Section 5, I'd like to know why there's an
>>> option here.  What effect is there on interoperability if an implementation
>>> chooses not to do what's described there?  I have the same question about
>>> the RECOMMENDED in Section 8.2, the RECOMMENDED in Section 8.6.2, the
>>> SHOULD in Section 8.7, the RECOMMENDED and SHOULD NOT in 8.7.1, and the
>>> SHOULD in 12.
>>>
>>>
>> Section 5: the reason for this SHOULD is explained in that paragraph, but
>> perhaps it isn't clear enough? Can you point me to the ambiguity? I'll
>> paraphrase: unsigned samples can be represented in signed samples in
>> several ways. As the coding tools provided by the FLAC format work best
>> with samples centered around zero (i.e. the highest compression is
>> reached), it is recommended to convert unsigned samples to signed samples
>> by subtracting half the range, if that indeed results in samples being
>> centered around zero. There is no interoperability concern when choosing
>> another method to convert unsigned samples to signed samples, it just
>> doesn't compress as good.
>>
>> Section 8.2: here I also do not understand the ambiguity, can you point
>> me to it? I'll paraphrase: FLAC is designed for audio, which has a sample
>> rate, otherwise it is no audio. If someone chooses to store something in
>> FLAC without a samplerate (i.e. something that is not audio), it is
>> recommended to add some metadata on what the data is. As I do not know of
>> these non-audio uses, the document cannot specify a format for this
>> metadata, so I can only recommend it, not enforce it.
>>
>> Section 8.6.2 is pretty much the same as 8.2, paraphrasing: if audio is
>> stored of which the specifics cannot be formally described in the FLAC
>> format, please store it informally in some way. This is deliberately
>> "open-ended". I'm unsure how to clarify this.
>>
>> Section 8.7: it says the bits have no meaning in a particular context. I
>> don't think the document should enforce these bits to be zero when they
>> have no meaning: a decoder should ignore these bits, not throw an error
>> when they are not zero.
>>
>> Section 8.7.1: the numbers should start with 1 and increase sequentially
>> because that makes sense to users. However, I think a parser should still
>> accept a cuesheet where this is not the case. This is because I know some
>> musicians have used cuesheet track numbers for artistic purposes in the
>> past, see for example: https://en.wikipedia.org/wiki/Hidden_track Or see
>> for example the album Follow the Leader by KoЯn, which starts on track 13.
>> Should I further clarify this?
>>
>> Section 12: The document says one SHOULD do something, except in certain
>> circumstances. If I change that to MUST, it doesn't work I think? Can a
>> MUST statement have exceptions written like this?
>>
>>
>>>
>>> Thanks for including Section 11.  Is the intent to include it after
>>> publication, or should the RFC Editor be asked to remove it?  Thanks also
>>> for Appendix B, which is useful for contex-setting for implementers.
>>>
>>>
>> The intention is to keep section 11 around. For that reason, a github
>> wiki is referenced, which can be properly updated.
>>
>>
>>>
>>> You might consider loading this into something that can pick off
>>> grammatical errors.  The ones I found are minor and the RFC Editor will
>>> certainly catch them, but it might make for a shorter AUTH48 if they're
>>> caught earlier.  For instance, in Section 9.2.7.2, there's "An odd
>>> folded residual value is gets shifted ...", wherein I believe the "is"
>>> should be dropped.
>>>
>>>
>> I will do that, thanks.
>>
>>
>>>
>>> Section 5 says "Older decoders MAY choose to abort decoding or skip data
>>> encoded using methods they do not recognize".  I thought having the "skip"
>>> option was curious, as this would allow potentially large or semantically
>>> significant gaps in the output of the decoder.  Isn't the "abort" option
>>> preferable?
>>>
>>>
>> Yes, probably. I'll change that.
>>
>> Kind regards, Martijn van Beurden
>>
>