Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add syntax of codecs parameter #78

Merged
merged 5 commits into from
Sep 16, 2020
Merged

Add syntax of codecs parameter #78

merged 5 commits into from
Sep 16, 2020

Conversation

nigelmegitt
Copy link
Contributor

@nigelmegitt nigelmegitt commented Sep 2, 2020

Also separate out the encouragement to support from the syntax definition and explanation as per discussion at #71 (comment), and add a note about MP4's codecs parameter.

Resolves #71.


Preview | Diff

@nigelmegitt nigelmegitt marked this pull request as ready for review September 2, 2020 11:01
@nigelmegitt nigelmegitt added the agenda Items for discussion in the next meeting label Sep 2, 2020
@mikedo
Copy link

mikedo commented Sep 3, 2020

  1. It would be good to add to the // comment about "element" that "period" is forbidden (analogous to the other comments).
  2. The new note about "the other" codecs parameter should better say: "Another codecs parameter is defined by [ RFC 6381 ] for application/mp4 (ISO BMFF) and extended for TTML by [ iso14496-30 ]."
  3. We should provide a concrete example (and not just the abstract ones). I offered one in my earlier comment.
  4. The formatting of the last line of the ABNF is broken into several lines.

index.html Outdated Show resolved Hide resolved
@css-meeting-bot
Copy link
Member

The Timed Text Working Group just discussed Add syntax of codecs parameter w3c/tt-profile-registry#78, and agreed to the following:

  • SUMMARY: @nigelmegitt to do 2nd editorial pass
The full IRC log of that discussion <nigel> Topic: Add syntax of codecs parameter #78
<nigel> github: https://github.com//pull/78
<nigel> Mike: 4 points, a couple are editorial.
<nigel> Nigel: My plan for the concrete example was to have a separate pull request.
<nigel> Mike: Sure, we should add it though, as we agreed last week.
<nigel> Nigel: Yes, no problem.
<nigel> Mike: Going through my comments.
<nigel> .. I thought it would be good to note that the element in RFC6381 prohibits "."
<nigel> .. It's not clear in RFC6381 but you can't add a "."
<nigel> Nigel: I wanted only to add a constraint statement and not duplicate what's in 6381.
<nigel> .. We could describe 6381 but not redefine it.
<nigel> Mike: I think 99% of readers won't go to 6381 so the prohibition of "." is worth calling out in a note.
<nigel> .. 2nd one is a bit of rewording.
<nigel> Nigel: Good for me.
<nigel> Mike: The concrete example using IMSC 1.0.1 and EBU-TT-D 1.0.1 would be valuable.
<nigel> .. And the 4th one is something about the formatting.
<nigel> Nigel: The formatting looks good to me.
<nigel> Cyril: It looks good to me too.
<nigel> Nigel: That's weird, I stole the formatting from IMSC to make it look the same.
<nigel> Cyril: There's a typo - I'll put a note on the line.
<nigel> Nigel: OK sounds like there's a bit of work, but straightforward, thanks for the reviews.
<nigel> SUMMARY: @nigelmegitt to do 2nd editorial pass

@nigelmegitt
Copy link
Contributor Author

  1. It would be good to add to the // comment about "element" that "period" is forbidden (analogous to the other comments).

I just traced this through, and I don't think "period" is forbidden.

The existing text in the TTML Profiles Registry says:

An identifier must conform with the element non-terminal of [RFC6381] and, furthermore, may not contain any of the characters in the regular expression character class [+|].

RFC6381 defines element as:

TOKEN is defined in [RFC2045]
...
element     := 1*octet-sim
octet-sim   := <any TOKEN character>

[RFC2045] defines:

     token := 1*<any (US-ASCII) CHAR except SPACE, CTLs,
                 or tspecials>

     tspecials :=  "(" / ")" / "<" / ">" / "@" /
                   "," / ";" / ":" / "\" / <">
                   "/" / "[" / "]" / "?" / "="
                   ; Must be in quoted-string,
                   ; to use within parameter values

...

Note that the definition of "tspecials" is the same as the RFC 822
   definition of "specials" with the addition of the three characters
   "/", "?", and "=", and the removal of ".".

By my reading, this means that token, and therefore element, is permitted to contain a period ".". @mikedo I guess this could be the subject of a separate issue, but for the time being I won't be adding a comment about period being forbidden. Happy to change my view if I've missed something here, of course.

@nigelmegitt
Copy link
Contributor Author

@mikedo @cconcolato I've addressed your review comments; I was left a bit confused by our discussion about the concrete example, since it also appears to be part of the proposal in #76; on balance I think it better addressed by a separate pull request there if that's okay.

The formatting problem appears to be an issue only with the HTML diff output, so no change needed there.

@mikedo
Copy link

mikedo commented Sep 3, 2020

It would be good to add to the // comment about "element" that "period" is forbidden (analogous to the other comments).

I just traced this through, and I don't think "period" is forbidden.

Period is a core delimiter in the 6381 codecs syntax. Element cannot contain a period. See 6381, section 3.4.

@nigelmegitt
Copy link
Contributor Author

Period is a core delimiter in the 6381 codecs syntax. Element cannot contain a period. See 6381, section 3.4.

That section does not define element though, it merely uses it. Section 3.2 defines element as noted above. I don't know if there was a different intent in 6381 that somehow didn't make it through, but the exclusion of "." from tspecials is called out explicitly in a note, so it looks like a very deliberate choice to permit it in element.

Oh hang on, I missed a crucial line two below the definition of octet-sim:

      element     := 1*octet-sim
      octet-sim   := <any TOKEN character>

                  ; Within a 'codecs' parameter value, "." is reserved
                  ; as a hierarchy delimiter

Since we do not want to allow hierarchy delimiters in the midst of identifiers, I agree we need to clarify that the intent for our codecs parameter is the same, i.e. prohibit ".".

@mikedo
Copy link

mikedo commented Sep 3, 2020

I was left a bit confused by our discussion about the concrete example, since it also appears to be part of the proposal in #76; on balance I think it better addressed by a separate pull request there if that's okay.

I don't understand the concern, but to remove all doubt about what I propose, please add the following real life examples somewhere:

For example, the codecs string for TTML documents that conform to both IMSC1.0.1 text profile and EBU-TT-D-1 (the intersection) would be "im1t|etd1". And for TTML documents that require both IMSC1.0.1 and EBU-TT-D-1 (the union) would be "im1t+etd1".

@nigelmegitt
Copy link
Contributor Author

OK @mikedo please could you have a look now, I think both remaining comments are addressed.

@nigelmegitt nigelmegitt removed the agenda Items for discussion in the next meeting label Sep 15, 2020
@nigelmegitt nigelmegitt merged commit 85f33fa into gh-pages Sep 16, 2020
@nigelmegitt nigelmegitt deleted the issue-0071-codecs branch September 16, 2020 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The codecs parameter should have a formal definition of the use of the combination operators.
4 participants