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

Regression from 1.0.2 to 1.1.0 #169

Closed
Robloche opened this issue Oct 17, 2019 · 17 comments
Closed

Regression from 1.0.2 to 1.1.0 #169

Robloche opened this issue Oct 17, 2019 · 17 comments
Assignees
Milestone

Comments

@Robloche
Copy link

Robloche commented Oct 17, 2019

Since I updated to version 1.1.0, I get a systematic crash trying to render my subtitles.
Here's an example.

I tried to pin down the culprit and reached this part of code (in html.js):
image

At this point, attr seems to be of an incorrect type and, of course, that leads to the following:
image

@palemieux
Copy link
Contributor

@Robloche Yes, imscJS should not die. However the document is also not a valid IMSC document: px units are used, e.g. at tts:fontSize="22px", but the root container extent is not specified in px -- see https://www.w3.org/TR/ttml-imsc1.1/#extent-root.

@palemieux
Copy link
Contributor

Duplicate from #168

@palemieux
Copy link
Contributor

@Robloche Should be fixed at #170

@Robloche
Copy link
Author

I'm not the one producing the TTMLs I use... So it's perfect if your lib can handle badly formed files.

Thanks!

@Robloche
Copy link
Author

I updated to 1.1.1 and still get the same error.
I understand the file I'm parsing is not valid but I thought version 1.1.1 would handle it anyway.
Am I mistaken?

@Robloche Robloche reopened this Oct 24, 2019
@palemieux
Copy link
Contributor

@Robloche Do you still see an uncaught exception?

@Robloche
Copy link
Author

Yes, precisely.

@palemieux
Copy link
Contributor

@Robloche I cannot replicate it at https://www.sandflow.com/imsc1_1/index.html using the file you provided above. Can you confirm you are using 1.1.1-rc.1?

@Robloche
Copy link
Author

With 1.1.1-rc.1, the uncaught exceptions are gone. Thanks!

But my subtitles are all messed up:
image

@palemieux
Copy link
Contributor

The document specifies tts:fontSize="22px". Since the author did not specify a root extent in pixels, it is not possible to determine what font size the author actually wanted, e.g. is it 22px relative to a 1080px or 720px or 480px tall image? Also the region height is set to 10%, which results in much clipping.

Can you DM me at pal@sandflow.com the source of file? It would be good to address the fundamental issue.

@Robloche
Copy link
Author

I tried to change the font size on-the-fly, by replacing 22px by a percentage but I still get the clipped vertical text.

Also, I DMed you a file.

@palemieux palemieux removed the PR open label Oct 25, 2019
@palemieux
Copy link
Contributor

@Robloche My guess is that the document was authored against 720p video. Adding the tts:extent="1280px 720px" attribute to the tt element seems to result in something usable.

@Robloche
Copy link
Author

I tried adding tts:extent="1280px 720px" to tt and now have this:

<tt xml:lang="fr" xmlns="http://www.w3.org/ns/ttml" xmlns:tts="http://www.w3.org/ns/ttml#styling" tts:extent="1280px 720px">
	<head>
		<metadata xmlns:ttm="http://www.w3.org/ns/ttml#metadata">
			<ttm:title>17378_16070_pigedecristal_hd</ttm:title>
			<ttm:desc>Converted by the Online Subtitle Converter developed by J. David Requejo - modified for diacritic support
			</ttm:desc>
		</metadata>
		<styling>
		 <style tts:displayAlign="center" tts:backgroundColor="transparent" tts:extent="100% 10%" tts:origin="0% 89%" tts:textAlign="center" tts:fontSize="22px" tts:fontFamily="proportionalSansSerif" xml:id="backgroundStyle"/>
		<style tts:backgroundColor="transparent" xml:id="speakerStyle" tts:textOutline="black 1px 0px" tts:color="white" style="backgroundStyle"/>
		</styling>
		<layout><region xml:id="region1" style="speakerStyle" /></layout>
	</head>
    ...

But i get the same result: clipped vertical text.

@palemieux
Copy link
Contributor

@Robloche Here's what I get:

Annotation 2019-10-29 082340

Do you see something different?

@Robloche
Copy link
Author

And here's what I get:
image

I inspected the DOM and saw this:
image

I don't know what's causing all these <SPAN>: my code or your lib?

@palemieux
Copy link
Contributor

@Robloche The library unfortunately needs to wrap spans and use the getBoundingClientRect()(https://developer.mozilla.org/en-US/docs/Web/API/Element/getBoundingClientRect) API call to detect line breaks, in order to polyfill subtitle/caption features not supported by web browsers. Are you using the library in a full web browser, or in a node.js environment, or something else? Also, is the containing div already in the DOM when calling imscJS?

@Robloche
Copy link
Author

Robloche commented Dec 4, 2019

I'm back again. :o)

So, I cleaned up some old CSS and got rid of my vertical stack of SPANs.

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

No branches or pull requests

2 participants