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.

Sign up for GitHub

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

Jump to bottom

Runtime error with version 1.1.0 #164

Closed
bbert opened this issue Sep 2, 2019 · 16 comments · Fixed by #165
Closed

Runtime error with version 1.1.0 #164

bbert opened this issue Sep 2, 2019 · 16 comments · Fixed by #165
Labels
1.1 bug PR open
Milestone
1.1.1 Release

Comments

@bbert
Copy link

bbert commented Sep 2, 2019

Hello,
It seems there is an issue with last version 1.1.0 of the library.
I integrated this version 1.1.0 in dash.js (current dash.js version is still using version 1.0.2) and I face a bug with the following TTML content:

<?xml version="1.0" encoding="utf-8"?>
<tt xmlns="http://www.w3.org/ns/ttml" xmlns:tt="http://www.w3.org/ns/ttml" xmlns:ttm="http://www.w3.org/ns/ttml#metadata" xmlns:ttp="http://www.w3.org/ns/ttml#parameter" xmlns:tts="http://www.w3.org/ns/ttml#styling" xml:lang="en">
<head>
	<metadata>
		<ttm:title/>
		<ttm:desc/>
		<ttm:copyright/>
	</metadata>
	<styling>
		<style xml:id="style0" />
	</styling>
	<layout>
		<region xml:id="region0" tts:extent="75% 15%" tts:origin="25% 85%" />
	</layout>
</head>
<body>
	<div>
		<p begin="435390:00:00.379" end="435390:00:01.338" region="region0" style="style0" xml:id="caption155450">Je sais ce que j&apos;ai à faire.<br/>-Vous devrez</p>
		<p begin="435390:00:01.618" end="435390:00:02.379" region="region0" style="style0" xml:id="caption155451">changer de ville.</p>
	</div>
</body>

At runtime:

image

With 1.0.2, there is no issue.

Can you check if it comes from the library or the content itself, which seems to be conform (I tested here: http://apps.sandflow.com/imscV/)
@palemieux
Copy link
Contributor

I am not able to reproduce the issue. Can you provide:

  • version of imscJS you are using
  • browser version
  • OS

P.S.: Something may have gone wrong in the copy-paste of the content since à is not correct in the french text.

@andreastai
Copy link

@bbert Possibly this is just a copy & paste error, but only to be sure this is not the cause: in the copied sample the closing </tt> tag is missing.

@bbert
Copy link
Author

bbert commented Sep 2, 2019

imscJS version: 1.1.0
browser version: chrome 76.0.3809.132
OS: Windows10

I made copy-paste directly from network preview of chrome devtools

Here is the original segment file:
tf1aas-textstream_11798664_fre=8000-1567403998379.zip

@bbert
Copy link
Author

bbert commented Sep 2, 2019

Yes for the missing </tt> it was a copy & paste error

@palemieux
Copy link
Contributor

I am running the same configuration and cannot replicate the error.

Perhaps your npm cache be corrupted? Could send over your html.js file?

@bbert
Copy link
Author

bbert commented Sep 2, 2019

html.zip

@palemieux
Copy link
Contributor

@bbert Thanks. The html.js file is identical to my copy. Thinking...

@bbert
Copy link
Author

bbert commented Sep 2, 2019

Have you tried with dash.js?

@palemieux
Copy link
Contributor

@bbert I do not currently have a dash.js environment setup. What is involved?

@bbert
Copy link
Author

bbert commented Sep 2, 2019

I was just wonder if it may be related with the way the library is integrated within dash.js. Thinking also...

@palemieux palemieux added the 1.1 label Sep 2, 2019
@bbert
Copy link
Author

bbert commented Sep 3, 2019

I have setup the dash.js reference sample with dash.js using version 1.1.0 of imscJS:
http://orange-opensource.github.io/dash.js/samples/dash-if-reference-player/index.html

I manage to reproduce with the sample stream "Subtitles and Captions / TTML Sideloaded XML Subtitles".

You can try yourself and do some debug.

The strange thing is that when you load for the 1st time the stream after having loaded/refeshed the page, then you get the error. But if you stop and (re)load the stream, then you don't get the error...

That's why I was thinking of a problem with imscJS library integration into dash.js.

@palemieux
Copy link
Contributor

palemieux commented Sep 3, 2019 •
edited
Loading

@bbert Thanks for setting up the sandbox!

It looks like element.getBoundingClientRect() returns an empty bounding rectangle on the first span that contains text (e).

Not sure why this would happen. Need to dig deeper, e.g. is the div onto which the subtitle rendered part of the DOM when renderHTML is called?

palemieux added a commit that referenced this issue Sep 3, 2019
@palemieux
Skip tts:textEmphasis processing on zero width spans (#164)
a62532d
@palemieux palemieux mentioned this issue Sep 3, 2019
Merged
@palemieux
Copy link
Contributor

@bbert I have created a PR at #165 . Can you give it a try?

@bbert
Copy link
Author

bbert commented Sep 4, 2019

@palemieux I tried and that works fine now. Thanks for the fix.

@palemieux
Copy link
Contributor

@bbert Any chance to deploy it on the sandbox? If not can you compare the rendering results from 1.0 to 1.1?

@bbert
Copy link
Author

bbert commented Sep 6, 2019

@palemieux

@palemieux palemieux added this to the 1.1.1 Release milestone Sep 8, 2019
palemieux added a commit that referenced this issue Sep 16, 2019
@palemieux
Skip tts:textEmphasis processing on zero width spans (#164)
f87aaa9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
1.1 bug PR open
Projects
None yet
Milestone
1.1.1 Release
Development

Successfully merging a pull request may close this issue.

Skip tts:textEmphasis processing on zero width spans sandflow/imscJS
3 participants
@andreastai @palemieux @bbert