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

Rename Glyph and Decoded Image Buffers #55

Merged
merged 7 commits into from
Jan 16, 2023
Merged

Conversation

palemieux
Copy link
Contributor

@palemieux palemieux commented Nov 7, 2022

Closes #54


Preview | Diff

@@ -192,7 +192,7 @@
documents that conform to any of the TTML Profiles for Internet Media Subtitles and Captions ([[IMSC]]).</p>

<p>The model is not intended as a specification of the processing requirements for implementations. For instance, while the
model defines a glyph buffer for the purpose of limiting the number of glyphs displayed at any given point in time, it neither
model defines a glyph buffers for the purpose of limiting the number of glyphs displayed at any given point in time, it neither

Choose a reason for hiding this comment

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

"a glyph buffers" -> "glyph buffers"

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.

I think we need to define [Decoded|Image|(blank)] [Back|Front] Buffer somewhere. The terms are used in a normative sense, but only explained informatively. That could be done in a separate issue and PR.

Related: reading this I'm getting slightly confused by repetition of "Back Buffer" and "Front Buffer" as I parse the sentences. I wonder if we should add a qualifier to the composited Back and Front Buffers, e.g. "Composition Back Buffer". Not sure if that'd be a net benefit, tbh!

I made and then deleted a couple of comments along the lines of "Front and Back are the wrong way round" for both the Glyph and Image Buffers, but looking at it harder, I realise that my confusion and uncertainty here is because the Back/Front model used for the composition Back Buffer and Front Buffer actually does not apply to the Glyph and Image Buffers. They are following a different pattern: rather than preparing in the Back Buffer, then transferring to the Front Buffer at presentation time, instead, they're a kind of Cache, where the stuff decoded for the previous non-empty ISD rendered into the Back Buffer remains available for the next one. There's actually a subtle difference in the cache handling for decoded images and for rendered glyphs:

The contents of the Decoded Image Buffer Dn SHALL be transferred instantaneously to Decoded Image Buffer Dn-1 at the presentation time of Intermediate Synchronic Document En.

vs

The contents of the Glyph Buffer Gn SHALL be copied instantaneously to Glyph Buffer Gn-1 at the presentation time of Intermediate Synchronic Document En.

(my emphasis)

I take transferred to mean moved, i.e. after the transfer, the buffer/cache is left empty, whereas copied means that afterwards the previous contents remain, and new contents can be added. That being the case, it's hard to understand why there are two buffers for Glyphs.

My conclusion from all this is that we should probably not use the terms Front Buffer and Back Buffer for these caches, because it doesn't actually help understanding, but that renaming them to reflect that they're Caches might be a worthwhile improvement.

Also, if the same approach to retention of cache contents is supposed to apply, we need to use the same phrasing.

@@ -447,11 +447,11 @@ <h2>Architecture</h2>

<p>To enable scenarios where the same glyphs are used in multiple successive <a data-lt=
"Intermediate Synchronic Document">Intermediate Synchronic Documents</a>, e.g. to convey a CEA-608/708-style roll-up (see
[[CEA-608]] and [[CEA-708]]), the Glyph Buffers G<sub>n</sub> and G<sub>n-1</sub> store rendered glyphs across <a data-lt=
[[CEA-608]] and [[CEA-708]]), the Glyph Front and Back Buffers store rendered glyphs across <a data-lt=
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure that the front- and back-ness of this is super helpful in the introduction. Suggestion for consideration:

Suggested change
[[CEA-608]] and [[CEA-708]]), the Glyph Front and Back Buffers store rendered glyphs across <a data-lt=
[[CEA-608]] and [[CEA-708]]), the Glyph Buffers store rendered glyphs across successive <a data-lt=

"Intermediate Synchronic Document">Intermediate Synchronic Documents</a>, allowing glyphs to be copied into the Presentation
Buffer instead of rendered, a more costly operation.</p>

<p>Similarly, Decoded Image Buffers D<sub>n</sub> and D<sub>n-1</sub> store decoded images across <a data-lt=
<p>Similarly, Decoded Image Front and Back Buffers store decoded images across <a data-lt=
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar suggestion to that for Glyph buffers here:

Suggested change
<p>Similarly, Decoded Image Front and Back Buffers store decoded images across <a data-lt=
<p>Similarly, Decoded Image Buffers store decoded images across successive <a data-lt=

@palemieux
Copy link
Contributor Author

@nigelmegitt I agree that the text could be clarified. Do you think replacing [Decoded Image|Glyph] [Back|Front] Buffer with [Decoded Image|Glyph] [Back|Front] Cache would be sufficient? Alternatively, we could also have a Decoded Image Cache and a Glyph Cache and, for each processed ISD, mark as retain those Decoded Image/Glyph that used so that all those that are not retain are purged across successive ISDs.

@nigelmegitt
Copy link
Contributor

@palemieux I like that suggestion that we retain or discard/purge from the cache. Sounds like it should be much clearer.

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.

I think this is going to end up much easier to understand; a few substantive points about the details.

spec/imsc-hrm.html Outdated Show resolved Hide resolved
spec/imsc-hrm.html Outdated Show resolved Hide resolved
Comment on lines 690 to 691
<p>It SHALL be an <a>error</a> if the sum of Normalized Image Area images stored in Decoded Image Cache is
greater than the Normalized Decoded Image Cache Size (NDIBS).</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

At what point in the process is this potential error condition checked? Would it not make sense to check which images are being reused, discard the remainder, and then attempt to decode the new image(s), and only then check if the error condition has been met? As it stands, all new images are decoded before discarding images that are not being reused.

spec/imsc-hrm.html Outdated Show resolved Hide resolved
<li>remove the <i>retain</i> flag from all remaining <a>glyphs</a> in the Glyph Cache; and</li>
</ol>

<p>It SHALL be an <a>error</a> for the sum of NRGA(g<sub>i</sub>) over all <a data-lt="glyph">glyphs</a> in the
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 for the image cache regarding when this error condition is checked, and whether glyphs for discard should be purged before drawing non-retained glyphs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at IMSC 1.2, the condition was imposed on the Glyph Buffer G_n, which I think corresponds to all glyphs flagged retain.

838e92f

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, that works I think. Not sure why the phrasing is different for glyphs and images, but it's a v minor point ("if the sum is" vs "for the sum to be").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason I can think of. Fixed.

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.

Thank you @palemieux!

@palemieux palemieux merged commit dd539ba into main Jan 16, 2023
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.

Rename Glyph Buffers Gn and Gn-1 and Decoded Image Buffers Dn and Dn-1
3 participants