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

consider removing recommendations that do not appear to address a technical problem #329

Closed
mikedo opened this issue Feb 6, 2018 · 3 comments
Assignees
Labels
Milestone

Comments

@mikedo
Copy link

mikedo commented Feb 6, 2018

I've run across a couple of "should" recommendations for things that do not appear to solve any problem, and in some cases discourage useful practices. And, validators tend to flag warnings if the recommendation is not followed, even though it is not necessary. Examples:

6.11 "If the Document Instance includes any length value that uses the c expression, ttp:cellResolution SHOULD be present on the tt element." Why? There is a well-defined initial value. If the author wishes to use that, what is the purpose of adding this attribute to set what is already set?

6.11 "All time expressions within a Document Instance SHOULD use the same syntax, either clock-time or offset-time." Why? Since decoders are required to handle both concurrently, what does this solve? It takes away the ability to mix setting "sync" points with clock-time and using offset-time for blocks (div) that can be reused in multiple places on the timeline.

I did not do an exhaustive analysis.

I believe removing these provisions would cause no harm. 1.0 documents would remain conformant, and decoders should not care since they already have to operate without the recommendations.

@andreastai
Copy link

6.11 "If the Document Instance includes any length value that uses the c expression, ttp:cellResolution SHOULD be present on the tt element." Why? There is a well-defined initial value. If the author wishes to use that, what is the purpose of adding this attribute to set what is already set?

I tend to agree. ttp:cellResolution is also relevant for tts:fontSize and tts:lineHeight. At the moment the recommendation applies only if ebutts:linePadding is used.

6.11 "All time expressions within a Document Instance SHOULD use the same syntax, either clock-time or offset-time." Why? Since decoders are required to handle both concurrently, what does this solve? It takes away the ability to mix setting "sync" points with clock-time and using offset-time for blocks (div) that can be reused in multiple places on the timeline.

I also tend to agree here. It is obvious to author something like the sample below:

<p begin="00:00:01.000" end="00:00:03.000">
  <span begin="0s">Hello </span>
  <span begin="1s">beautiful </span>
  <span begin="2s">world!</span>
 </p>

If this would be something to change in 1.0.1 or if this should be done in 1.1 is a different question.

@css-meeting-bot
Copy link
Member

The Working Group just discussed consider removing recommendations that do not appear to address a technical problem imsc#329, and agreed to the following resolutions:

  • RESOLUTION: Remove the recommendation to include cellResolution in 1.1
  • RESOLUTION: Remove the recommendation to use the same time expression syntax throughout a document in 1.1.
The full IRC log of that discussion <nigel> Topic: consider removing recommendations that do not appear to address a technical problem imsc#329
<nigel> github: https://github.com//issues/329
<nigel> Mike: I ran across the spec, and there were two SHOULD provisions about authoring and
<nigel> .. they don't seem to solve any problem and put unnecessary constraints on the authoring,
<nigel> .. at least at the recommendation level, and that seems weird to me.
<nigel> Nigel: I believe the cellResolution SHOULD is because we did not want to rely on default values.
<nigel> Mike: Then lots of other default values need additional text.
<nigel> Andreas: In this case the cell resolution is used even when the c unit is not used.
<nigel> Glenn: I suspect it was done by analogy to pixel, where there is no default extent on the root.
<nigel> .. In this case I agree it is unnecessarily overconstrained.
<nigel> Pierre: It traces history way back including EBU-TT-D.
<nigel> Andreas: I see both sides - especially on cellResolution a general recommendation to set it
<nigel> .. can make sense.
<nigel> Pierre: The reason it was put in there was because at some point in an early EBU-TT-D draft
<nigel> .. it was mandatory.
<nigel> Mike: But it is not now.
<nigel> Pierre: There's a desire for EBU-TT-D to be a subset. That meant we could not prohibit it in IMSC1.
<nigel> Nigel: Can I propose to remove this cellResolution recommendation?
<nigel> Pierre: No objection for IMSC 1.1. We tried to minimise changes to IMSC 1.0.1 and if we
<nigel> .. change it there then some validators will complain wrongly.
<nigel> Andreas: I would also go with that. There have been liaisons exchanged with other groups
<nigel> .. and I'm not sure about the other standards organisations' responses to changing 1.0.1.
<nigel> .. It could be viewed as a substantive edit.
<nigel> Mike: I don't feel strongly about acting on this in 1.0.1.
<nigel> RESOLUTION: Remove the recommendation to include cellResolution in 1.1
<nigel> Mike: Moving on to the second one about time expressions using the same syntax.
<nigel> .. There are good authoring reasons not to conform with this.
<nigel> Andreas: I made a comment also, agreeing with this.
<nigel> Nigel: I can't recall why this is present.
<nigel> Pierre: This came from CFF-TT - I can only assume that it was an attempt to stop authors
<nigel> .. from making things more complicated. I'm not aware of any technical reason for this constraint.
<nigel> Mike: I do not recall anything either.
<nigel> Pierre: I think it can be removed.
<nigel> RESOLUTION: Remove the recommendation to use the same time expression syntax throughout a document in 1.1.
<nigel> Mike: I'll change the label to 1.1.
<nigel> Pierre: I'll prepare a pull request.

@nigelmegitt
Copy link
Contributor

This should have been closed when #330 was merged, not sure why it wasn't. Thanks for picking it up @palemieux .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants