-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix(CEA): reset PTS on new init segment #6606
Conversation
Incremental code coverage: 100.00% |
Fixes #6605 Resets cache (including last used presentation timestamp) of CEA Decoder on every init segment append. Adds few debug logs to easify investigations in the future.
shaka.log.debug('Passing new init segment to CEA parser'); | ||
// Reset underlying decoder when new init segment arrives | ||
// to clear stored pts values. | ||
this.reset(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this cause undesirable behavior on a bit-rate change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you be more specific? It seems to be the explicit intent of the author to reset CEA decoding when we switch bitrates (append init segment). What undesirable behavior are you seeing / anticipating?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issues addressed is "Closed Captions from main content appear in the end of midroll" which indicates to me a concern with period changes, not bitrate changes. I think this change might cause captions to disappear earlier than they should on a bitrate change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think they need to distinguish period change from bitrate change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right. @caridley can you send a PR to fix it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I will have time to do that. Sorry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a fix for it, #6671
Fixes #6605 Resets cache (including last used presentation timestamp) of CEA Decoder on every init segment append. Adds few debug logs to easify investigations in the future.
Fixes #6605
Resets cache (including last used presentation timestamp) of CEA Decoder on every init segment append.
Adds few debug logs to easify investigations in the future.