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

Incorrect rendering of CEA608 captions #1975

Closed
squarebracket opened this issue May 30, 2017 · 10 comments · Fixed by #1981
Closed

Incorrect rendering of CEA608 captions #1975

squarebracket opened this issue May 30, 2017 · 10 comments · Fixed by #1981
Assignees

Comments

@squarebracket
Copy link
Contributor

squarebracket commented May 30, 2017

Environment
Observed behaviour

Nightly now incorrectly renders the size of CEA608 captions when using the TTML renderer. For the CEA608 test stream, it renders correctly when at full size. However, opening the console cuts off the bottom of the captions div:
screenshot from 2017-05-30 09-25-14

Having the console closed is obviously the normal use case, but I get much bigger problems with real-world captions:
screenshot from 2017-05-30 09-31-20
screenshot from 2017-05-30 09-26-52

Captions seem to have enough space when viewing the stream in fullscreen:
screenshot from 2017-05-30 09-46-18

Do I now need to manually set fontSize or something? Or is this just a bug in determining video size when not fullscreen?

@TobbeEdgeware TobbeEdgeware self-assigned this May 30, 2017
@TobbeEdgeware
Copy link

It is probably scaleCue in TextTracks.js which is not being properly called. I'll try to look into it.

@nicosang
Copy link
Contributor

I don't know CEA608 very well, but @TobbeEdgeware what is the meaning of this values?

aspectRatio = 3.5 / 3.0;
use80Percent = true;

with those values, the getVideoVisibleVideoSize function returns this...
cea608

it seems the videoCaptionContainer has not the good aspect. So, according to me, the issue is not the scaleCue but rather the checkVideoSize : do you agree?

@nicosang
Copy link
Contributor

nicosang commented Jun 1, 2017

@squarebracket could you, please, have a look to my PR? does it solve your issue?

@squarebracket
Copy link
Contributor Author

squarebracket commented Jun 2, 2017

I'm not sure about the aspectRatio, but CEA608 says the safe zone (i.e. where the captions are to appear) is 80% of the display.

As for your PR, it definitely made it better, in that they're now displaying in the right area and at the right size again. The tails of some characters are still a bit chopped off, though:
screenshot from 2017-06-01 21-45-15

Setting the CSS of #video-caption to be overflow: visible, as you'd expect, makes that problem disappear. I'm not sure if you want to spend the time pixel-fixing the bounding box to perfection, but IMO setting overflow: visible is totally reasonable since the captions are appearing in the right spot.

@squarebracket
Copy link
Contributor Author

Oh, I see in the WebVTT spec that you're supposed to have overflow: hidden. Hmm.

@nicosang
Copy link
Contributor

nicosang commented Jun 2, 2017

@squarebracket could you, please, test with an older dash.js version? a version before imsc1.js integration....it's probably a cea608 issue....
in the EmbeddedTextHtmlRender file, lineHeight and linePadding are always empty.... :-(

@TobbeEdgeware
Copy link

@nicosang Regarding the aspect ratio, CEA-608 was designed as 80% for 4:3 aspect ratio, and it does not look good to extend to 80% for 16:9 content. The aspect ratio

aspectRatio = 3.5 / 3.0;
use80Percent = true;

were heuristically chosen to look good, but may need to be changed. If there is another value that is better, please feel free to suggest a change.

@nicosang
Copy link
Contributor

nicosang commented Jun 2, 2017

As I previously said, I'm not an expert of cea608. My question was to help me to understand the problem. So, I prefer to keep those values ;-). Thanks for your answer.
@TobbeEdgeware, have you an idea on the chopped off characters issue? Can we have from ce608 parser lineHeight and linePadding values?

@squarebracket
Copy link
Contributor Author

squarebracket commented Jun 2, 2017

@nicosang I see the tails chopped off on 2.5.0 as well.

As far as lineHeight and linePadding. CEA608 doesn't carry padding information. The only line information it has is that it's a grid of 15 rows and 32 columns (inside that 80% window as @TobbeEdgeware mentioned).

One interesting thing to note. The tails only seem to be cut off when there's two or more lines displayed at once. A single line, I can see the tails. Maybe there's something wonky with the rounding of the region size here.

@TobbeEdgeware
Copy link

If I remember correctly, each line is its own region in order to be able to have lines which are not adjacent, without needing empty lines in between. One can probably tune these regions together with the fonts to make things look better and the formulas that @squarebracket points out is probably an important part of this.

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 a pull request may close this issue.

3 participants