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

Port IMSC1.0.1 fixes #322

Merged
merged 14 commits into from
Mar 8, 2018
Merged

Port IMSC1.0.1 fixes #322

merged 14 commits into from
Mar 8, 2018

Conversation

palemieux
Copy link
Contributor

@palemieux palemieux commented Feb 5, 2018

@nigelmegitt
Copy link
Contributor

@palemieux I'm assuming from the "[WIP]" label that there's more to do, so will hold off reviewing.

@palemieux palemieux closed this Feb 6, 2018
@palemieux palemieux reopened this Feb 6, 2018
@palemieux palemieux changed the title Port IMSC1.0.1 fixes [WIP] Port IMSC1.0.1 fixes Feb 6, 2018
Copy link
Contributor

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please could you check the merge conflicts were resolved correctly @palemieux ?

@@ -392,7 +394,7 @@ <h4>Override</h4>
<h2>Supported Features and Extensions</h2>

<p>See <a href="#conformance"></a> for a definition of <em>permitted</em>, <em>prohibited</em>, <em>optional</em> and
<em>permitted-deprecated</em>.</p>
<em>permitted-deprecated</em>.</p>&lt;&lt;&lt;&lt;&lt;&lt;&lt; HEAD
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a leftover from a (possibly unresolved conflict).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sigh. Leftover from incomplete delete....

Copy link
Contributor

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One "this doesn't do what I expected" editorial point about indentation in the example, and a query about whether profile signalling should still reference 1.0.1 or should be updated as part of the porting to point to 1.1.

</p>
</div>
</body>
<head>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't match the editorial tweak made at b70025d which reduced the indentation spacing.

<li>one <code>ebuttm:conformsToStandard</code> element, as specified in [[!EBU-TT-D]] and illustrated in <a href=
"#ebu-tt-d-interop"></a>, SHOULD be set to the [[ttml-imsc1.0.1]] profile designator corresponding to the profile to
which the <a>Document Instance</a> conforms;
<li>the designator of the [[ttml-imsc1.0.1]] profile to which the <a>Document Instance</a> conforms and the URI
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be updated for IMSC 1.1 rather than pointing at IMSC1.0.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Authors that really want EBU-TT-D compatibility should stick to IMSC1.0.1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we explicitly say that IMSC 1.1 processors must be able to handle imc1.0.1 designated documents? Presumably it would be reasonable to include both the 1.0.1 and the 1.1 designators - are we recommending that is not done?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably it would be reasonable to include both the 1.0.1 and the 1.1 designators - are we recommending that is not done?

The specification neither discourages nor encourages the presence of the 1.1 designator -- certainly it is not prohibited. I was thinking of avoiding mentioning the 1.1 designator to make things simpler for authors, and avoid anyone believing that any 1.1 document is compatible with EBU-TT-D.

I am not going to object also including the 1.1 designator if you feel extremely strongly about it. Let me know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just think it seems strange not to mention the 1.1 designator since this is the 1.1 spec, so I would add it, yes. Otherwise it makes it seem like IMSC 1.1 documents should not declare themselves as such, but as 1.0.1 documents. Whereas they can reasonably declare themselves to be both.

which the <a>Document Instance</a> conforms;
<li>the designator of the [[ttml-imsc1.0.1]] profile to which the <a>Document Instance</a> conforms and the URI
<code>urn:ebu:tt:distribution:2014-01</code> SHOULD each be carried in an <code>ebuttm:conformsToStandard</code> element
as specified in [[!EBU-TT-D]] and illustrated in <a href="#ebu-tt-d-interop"></a>.
</li>
</ul>

<p class="note">If <code>ebuttm:conformsToStandard</code> elements signal neither [[ttml-imsc1.0.1]] nor [[!EBU-TT-D]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, should this be updated to say IMSC 1.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Authors that really want EBU-TT-D compatibility should stick to IMSC1.0.1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as #322 (comment)

@palemieux palemieux added this to the imsc1.1 CR milestone Feb 9, 2018
Copy link
Contributor

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to go round the loop on this again, the section on EBU-TT-D compatibility still reads strangely to me (though better), and has at least one grammatical issue in note text.

@@ -2684,15 +2702,28 @@ <h4>EBU-TT-D Compatibility</h4>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first paragraph only refers to IMSC 1.0.1; in the context of IMSC 1.1 that looks strange. Since IMSC 1.0.1 already defines this information, we probably should clarify that conformance to 1.0.1 Text profile also implies conformance to IMSC 1.1 Text profile. So suggest rewording to:

<p>If the <a>Document Instance</a> conforms to both [[ttml-imsc1.0.1]] and [[EBU-TT-D]] (and therefore also conforms to the <a>Text Profile</a>), and compatibility with

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably should clarify that conformance to 1.0.1 Text profile also implies conformance to IMSC 1.1 Text profile.

I think this is already stated many times before.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not happy here, but I think the change needed is bigger. I'll open a pull request against the branch that this pull request seeks to merge, which is the best way to demonstrate what I think needs to happen.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've raised a proposal as pull request #337, whose merge target is the source branch of this pull request.

"#ebu-tt-d-interop"></a>, SHOULD be set to the [[ttml-imsc1.0.1]] profile designator corresponding to the profile to
which the <a>Document Instance</a> conforms;
<li>the value of one <code>ebuttm:conformsToStandard</code> element is equal to the designator of the [[ttml-imsc1.0.1]]
profile to which the <a>Document Instance</a> conforms;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot recall why we are not more prescriptive here about the IMSC 1.0.1 Text Profile designator, for example by including it directly as the value to use. Any reason why not?

Also why do we leave it open that it could potentially be the Image Profile, when we state in §I.2 that it cannot?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nigelmegitt As noted in §I.2, there is a corner case where IMSC 1.0.1 Image Profile can conform to EBU-TT-D.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks @palemieux

</li>

<li>the value of one <code>ebuttm:conformsToStandard</code> element is equal to the designator of the <a>Text Profile</a>
or <a>Image Profile</a> to which the <a>Document Instance</a> conforms; and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to §I.2 it cannot conform to the Image Profile as well as EBU-TT-D. If this is to allow for the degenerate case of an empty document then I'd prefer to clarify that, for example by changing or <a>Image Profile</a> to:

(or, if applicable e.g. for an empty document, <a>Image Profile</a>)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The solution is to move 7.9.3 into I.2, to avoid repeating provisions and forcing the reader to toggle between two sections. I propose addressing this as part of #331 . This PR has grown past its original scope.

</li>

<li>the value of one <code>ebuttm:conformsToStandard</code> element is equal to the URI
<code>urn:ebu:tt:distribution:2014-01</code>.</li>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In support of my comment on line 2706, it seems inconsistent to copy the EBU-TT-D URI here but not the IMSC 1.0.1 URIs there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EBU-TT-D does not specify the urn:ebu:tt:distribution:2014-01 identifier (anymore?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that is pending publication of EBU-TT-D 1.0.1, which itself depends on the publication of IMSC 1.0.1 as a Rec. The EBU's pattern is to include a month indicator in the URN. I'll see if we can resolve this circularity with EBU so that we can make an update to this before moving to Rec of IMSC 1.0.1.

of this section.</p>

<p class="note">If none of the <code>ebuttm:conformsToStandard</code> elements specified above are present, the <a>Document
Instance</a> conforms is communicated through means outside the scope of this specification, e.g. through the <a>document
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar point: "the Document Instance conforms is communicated" doesn't make sense - I think this is supposed to be:

the profiles to which the Document Instance conforms are communicated

@palemieux palemieux merged commit 3212412 into IMSC1.1 Mar 8, 2018
@palemieux palemieux deleted the issue-292-port-imsc101-issues branch April 24, 2018 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants