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

Improve PES ACC overflow handling #2530

Merged
merged 1 commit into from Feb 26, 2020
Merged

Conversation

robwalch
Copy link
Collaborator

@robwalch robwalch commented Feb 17, 2020

This PR will...

Fixes an issue where incomplete AAC frames at the end of PES packets are skipped.

Why is this Pull Request needed?

Fixes loss of data and corruption of certain media streams. (#2528)

Are there any points in the code the reviewer needs to double check?

No.

Resolves issues:

Fixes #2528

Checklist

  • changes have been done against master branch, and PR does not conflict

@robwalch robwalch added this to the 0.13.2 milestone Feb 17, 2020
@robwalch robwalch added this to Top priorities in Release Planning and Backlog Feb 17, 2020
@robwalch robwalch force-pushed the bugfix/ts-pes-aac-overflow branch 2 times, most recently from 7ea3566 to f2ac973 Compare February 25, 2020 23:21
// logger.log('Unable to parse AAC frame');
break;
if (ADTS.isHeader(data, offset)) {
if ((offset + 5) < len) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand that the case is such because ADTS.isHeader doesn't check that the whole 7 or 9 bytes exist with or without CRC signature. However, i'm not sure why the magic number is + 5 from offset. Since this was the value before it doesn't block this fix obviously.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The magic number should be the theoretical size of the smallest ADTS packet (header plus frame header). My guess is it’s only large enough to guard against exceptions when calling her frame.

@robwalch robwalch merged commit 4f084c9 into master Feb 26, 2020
@robwalch robwalch deleted the bugfix/ts-pes-aac-overflow branch February 26, 2020 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Release Planning and Backlog
  
Top priorities
Development

Successfully merging this pull request may close these issues.

Chrome: PIPELINE_ERROR_DECODE not seen on other browsers
2 participants