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

Reversed cue display order for identical time ranges #848

Closed
joeyparrish opened this issue Jun 5, 2017 · 6 comments
Closed

Reversed cue display order for identical time ranges #848

joeyparrish opened this issue Jun 5, 2017 · 6 comments
Assignees
@joeyparrish
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone
v2.2.0

Comments

@joeyparrish
Copy link
Member

Split from #821 filed by @ewosleger:

Shaka v2.0.8
We noticed Shaka is displaying our captions in reverse order from our embedded ttml files in the manifest:
"jumped over the fence"
"Quick brown fox"

Regarding our captions in reverse order. A snippet of a specific ttml segment.
<p begin="00:00:55.000" end="00:00:56.541" region="pop1" style="basic" tts:extent="45% 5.33%" tts:origin="27.5% 79.33%"> JEANNIE: <span tts:fontStyle="italic">You still</span> </p> <p begin="00:00:55.000" end="00:00:56.541" region="pop2" style="basic" tts:extent="67.5% 5.33%" tts:origin="15% 84.66%"> <span tts:fontStyle="italic">stubbornly insist on being</span> </p>

The first <p> "pop1" tss:origin y cord is 79.33% from the top.
The secons <p> "pop2" tss:origin y cord should be below "pop1" y at 84.66% however it is rendering above the first <p>.

screen shot 2017-05-23 at 2 18 41 pm
@joeyparrish joeyparrish mentioned this issue Jun 5, 2017
Closed
@ismena ismena added type: external An issue with an external dependency; not our issue; sometimes kept open for tracking status: waiting on response Waiting on a response from the reporter(s) of the issue and removed needs triage labels Jun 5, 2017
@ismena ismena self-assigned this Jun 5, 2017
@ismena
Copy link
Contributor

ismena commented Jun 5, 2017

As mentioned in comments to #821 I verified that we are parsing the "tts:origin" attribute and constructing the VttCues correctly, so it's likely to be a browser bug.
@ewosleger, if you let me know which browser(s) is/are affected I could verify the assumption and file bugs on them if need be.

@ismena ismena added the type: bug Something isn't working correctly label Jun 5, 2017
@kjnsn
Copy link

kjnsn commented Aug 7, 2017 •
edited

So the browser is actually doing what the spec says: https://w3c.github.io/webvtt/#webvtt-cue-line-automatic

If all lines are "auto", the first cue will be given the line -1, the second -2, and so on. This leads to the captions appearing reversed. I've checked dash.js and it has the same issue.

@joeyparrish joeyparrish removed type: external An issue with an external dependency; not our issue; sometimes kept open for tracking status: waiting on response Waiting on a response from the reporter(s) of the issue labels Aug 7, 2017
@joeyparrish joeyparrish assigned joeyparrish and unassigned ismena Aug 7, 2017
@joeyparrish joeyparrish added this to the v2.2.0 milestone Aug 7, 2017
@joeyparrish
Copy link
Member Author

If the browser behavior is to-spec, then we should reverse the order of the cues before sending them to the browser.

shaka-bot pushed a commit that referenced this issue Aug 7, 2017
@joeyparrish
Fix display order of cues with identical ranges
9d47a78 
Since the browser displays identical ranges from bottom up, we must
reverse the order before feeding them to the browser.  This way, the
first one parsed shows up on top.

Issue #848

Change-Id: Id2e6582e610808f7061bd0f8281c0705ecf1d6dc
@joeyparrish
Copy link
Member Author

We've pushed a fix to master for the case where line=auto, but we will leave this issue open a little longer until we can determine what the issue is in other cases.

@joeyparrish
Copy link
Member Author

I'm unable to reproduce any issue in Chrome's support of line=.

I've tested line=-1, -2, -3 with snapToLines=true and line=10, 20, 30 with snapToLines=false. In all cases, in any insertion order, the display is correct.

If you see any further issue with this, please let us know and we can reopen.

joeyparrish added a commit that referenced this issue Aug 7, 2017
@joeyparrish
Fix display order of cues with identical ranges
fb51fe4 
Since the browser displays identical ranges from bottom up, we must
reverse the order before feeding them to the browser.  This way, the
first one parsed shows up on top.

Issue #848

Backported to v2.1.x
@joeyparrish
Copy link
Member Author

joeyparrish commented Aug 7, 2017 •
edited

Fix backported to v2.1.6.

shaka-bot pushed a commit that referenced this issue Aug 10, 2017
@TheModMaker
Fix text tracks on IE/Edge.
404d42c 
IE/Edge don't accept cues that are out of time order.  So when we
reverse the order of cues for #848, the cues would be rejected.  Now
we sort the cues and only reverse the order of cues with equal times.

Change-Id: I860e1ea9694eb95ff2e74d9545c92373eb371686
joeyparrish pushed a commit that referenced this issue Aug 10, 2017
@TheModMaker @joeyparrish
Fix text tracks on IE/Edge.
87b40b1 
IE/Edge don't accept cues that are out of time order.  So when we
reverse the order of cues for #848, the cues would be rejected.  Now
we sort the cues and only reverse the order of cues with equal times.

Backported to v2.1.x

Change-Id: I860e1ea9694eb95ff2e74d9545c92373eb371686
@joeyparrish joeyparrish mentioned this issue Aug 14, 2017
Closed
@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@joeyparrish joeyparrish

Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Milestone
v2.2.0
Development

No branches or pull requests
4 participants
@ismena @joeyparrish @kjnsn @shaka-bot