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

WEBVTT - Text Alignment values missing from output #925

Closed
vish91 opened this issue Mar 25, 2021 · 4 comments · Fixed by #924
Closed

WEBVTT - Text Alignment values missing from output #925

vish91 opened this issue Mar 25, 2021 · 4 comments · Fixed by #924
Assignees
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@vish91
Copy link
Contributor

vish91 commented Mar 25, 2021

Issue and steps to reproduce the problem

Packager Command:
To reproduce from master branch try running on test input

$ packager 'in=SampleVideo_1280x720_30mb.mp4,stream=audio,segment_template=/tmp/Sample_video_1280x720_hls/seg_$Number$.aac,lang=en,hls_name=English' 'in=1835844163863-web.vtt,stream=text,language=en,segment_template=/tmp/1835844163863-ttml-hls/seg_$Number$.vtt,hls_group_id=cbsi_vtt,hls_name=English' \
--segment_duration 6 --hls_master_playlist_output /tmp/SampleVideo_1280x720_30mb_hls/master.m3u8

[0324/172546:INFO:demuxer.cc(89)] Demuxer::Run() on file '1835844163863-web.vtt'.
[0324/172546:INFO:demuxer.cc(155)] Initialize Demuxer for file '1835844163863-web.vtt'.
[0324/172546:INFO:demuxer.cc(89)] Demuxer::Run() on file 'SampleVideo_1280x720_30mb.mp4'.
[0324/172546:INFO:demuxer.cc(155)] Initialize Demuxer for file 'SampleVideo_1280x720_30mb.mp4'.
[0324/172549:INFO:mp4_media_parser.cc(345)] Ignore unused 'mdat' box - this could be as a result of extra not usable 'mdat' or 'mdat' associated with unrecognized track.
Packaging completed successfully.
$ cat /tmp/1835844163863-ttml-hls/seg_1.vtt
WEBVTT
X-TIMESTAMP-MAP=LOCAL:00:00:00.000,MPEGTS:9000

00:00:03.670 --> 00:00:04.462 line:85% position:28% size:35%
>> Devon: Hey.

00:00:04.546 --> 00:00:05.088 line:79% position:10% size:73% align:start
>> Amanda: Naya backed out on
our meeting.

00:00:05.171 --> 00:00:07.424 line:85% position:43% size:65%
>> Devon: Did she say why?

with this fix

$packager 'in=SampleVideo_1280x720_30mb.mp4,stream=video,segment_template=/tmp/SampleVideo_1280x720_30mb_hls/seg_$Number$.ts,iframe_playlist_name=SampleVideo_1280/stream_iframe.m3u8,playlist_name=SampleVideo1280x720/stream.m3u8'\
'in=SampleVideo_1280x720_30mb.mp4,stream=audio,segment_template=/tmp/Sample_video_1280x720_hls/seg_$Number$.aac,lang=en,hls_name=English' 'in=1835844163863-web.vtt,stream=text,language=en,segment_template=/tmp/1835844163863-ttml-hls/seg_$Number$.vtt,hls_group_id=cbsi_vtt,hls_name=English' \
--segment_duration 6 --hls_master_playlist_output /tmp/SampleVideo_1280x720_30mb_hls/master.m3u8
[0324/172855:INFO:demuxer.cc(89)] Demuxer::Run() on file '1835844163863-web.vtt'.
[0324/172855:INFO:demuxer.cc(155)] Initialize Demuxer for file '1835844163863-web.vtt'.
[0324/172855:INFO:demuxer.cc(89)] Demuxer::Run() on file 'SampleVideo_1280x720_30mb.mp4'.
[0324/172855:INFO:demuxer.cc(155)] Initialize Demuxer for file 'SampleVideo_1280x720_30mb.mp4'.
[0324/172859:INFO:mp4_media_parser.cc(345)] Ignore unused 'mdat' box - this could be as a result of extra not usable 'mdat' or 'mdat' associated with unrecognized track.
Packaging completed successfully.

$ cat /tmp/1835844163863-ttml-hls/seg_1.vtt
WEBVTT
X-TIMESTAMP-MAP=LOCAL:00:00:00.000,MPEGTS:9000

00:00:03.670 --> 00:00:04.462 line:85% position:28% size:35% align:center
>> Devon: Hey.

00:00:04.546 --> 00:00:05.088 line:79% position:10% size:73% align:start
>> Amanda: Naya backed out on
our meeting.

00:00:05.171 --> 00:00:07.424 line:85% position:43% size:65% align:center
>> Devon: Did she say why?

What is the expected result?

  • Expected VTT files should carry over styling and text properties based on the WEBVTT spec and input file
    What happens instead?
  • Outputs omits certain properties.
@vish91
Copy link
Contributor Author

vish91 commented Mar 25, 2021

@kqyang found a small bug. Please take a look when you can.
It's causing problems with mainly older version of ExoPlayer or other players who do not handle default webvtt text alignment missing on cue's.
Looking into the webvtt parser, the parser is doing things correctly just the webvtt utils that should be writing these bytes to out stream was missing what to do when text alignment is center.
PR linked ^

@kqyang
Copy link
Contributor

kqyang commented Mar 25, 2021

Thanks @vish91 for troubleshooting and fixing the problem.

@TheModMaker Can you review @vish91's PR?

@kqyang kqyang added type: bug Something isn't working correctly and removed needs triage labels Mar 25, 2021
@shaka-bot shaka-bot added this to the v2.5 milestone Mar 25, 2021
@vish91
Copy link
Contributor Author

vish91 commented Mar 25, 2021

@kqyang and @TheModMaker some test cases have failed, I'll update that. Before I fix that, correct me if I am wrong here but I think the parser is initially written to consider align:center as default so it doesn't add align:center to outputs packager generates.
Which means we are not considering the fact that for backwards compatibility if my input vtt has align:center , my output files should have it too instead it's getting ignored today. So this change will add align:center which will fix the legacy player issues that don't automatically align because they still are getting align:middle instead of align:center or not seeing an alignment property and don't know what to make of it.
https://github.com/google/ExoPlayer/blob/release-v2/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCueParser.java#L459-L478

@vish91
Copy link
Contributor Author

vish91 commented Apr 2, 2021

@kqyang @TheModMaker any thoughts ?

kqyang pushed a commit that referenced this issue May 5, 2021
Legacy players, e.g. older versions of ExoPlayer, do not handle default webvtt text alignment correctly. Need to specify `align:center` explicitly cues without text alignment for backwards compatibility.

Fixes #925.
nvincen pushed a commit to nvincen/shaka-packager that referenced this issue Jun 8, 2021
Legacy players, e.g. older versions of ExoPlayer, do not handle default webvtt text alignment correctly. Need to specify `align:center` explicitly cues without text alignment for backwards compatibility.

Fixes shaka-project#925.
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Jul 4, 2021
@shaka-project shaka-project locked and limited conversation to collaborators Jul 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants