Skip to content

PGS subtitle decoding support - #3673

Merged
ojw28 merged 1 commit into
google:dev-v2from
drhill:dev-v2_PGS
Jan 15, 2018
Merged

ojw28 merged 1 commit into
google:dev-v2from
drhill:dev-v2_PGS

Conversation

@drhill

@drhill drhill commented Jan 6, 2018

Copy link
Copy Markdown
Contributor

Where can I send a sample file? 83MB mkv file

@ojw28

ojw28 commented Jan 8, 2018

Copy link
Copy Markdown
Contributor

Is it possible for you to host it somewhere and send a link (to dev.exoplayer@gmail.com)? Thanks.

@drhill

drhill commented Jan 8, 2018

Copy link
Copy Markdown
Contributor Author

Email sent

@ojw28

ojw28 commented Jan 9, 2018

Copy link
Copy Markdown
Contributor

Got it, thanks.

int index = 0;
for (Holder curr : list) {
cues[index] = curr.build();
cueStartTimes[index++] = curr.start_time;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: start_time is never assigned, and so is always 0. Does the PGS spec actually carry timestamps, or are the timestamps always provided out of band (i.e. elsewhere in the container)?

  • If it can carry timestamps then there's something missing in this implementation.
  • If they're always provided out of band then it's guaranteed that there's only a single event time for each sample, and hence there's no reason to pass any timestamps to PgsSubtitle. This allows PgsSubtitle to be simplified down to something that looks identical to DvbSubtitle.

No need to update this pull request as I'm going through tweaking it a bit anyway, but please clarify (same will apply to any other comments). Thanks!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I could tell the time stamps are only provided out of band, but I've only used this with MKV files. It would make sense to simplify this. It's been awhile since I wrote this. I think I was modeling it off of SubripDecoder/SubripSubtitle class at the time.

@ojw28
ojw28 merged commit dc38e86 into google:dev-v2 Jan 15, 2018
@drhill
drhill deleted the dev-v2_PGS branch January 24, 2018 22:32
@google google locked and limited conversation to collaborators Jun 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants