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

Jump to bottom

tsdemuxer: if PES does not contain PTS/DTS, use last PES PTS/DTS instead #2109

Merged
merged 1 commit into from Jan 9, 2020
Merged

tsdemuxer: if PES does not contain PTS/DTS, use last PES PTS/DTS instead #2109

merged 1 commit into from Jan 9, 2020

Conversation

mangui
Copy link
Member

@mangui mangui commented Jan 30, 2019

This PR will ...

attempt to fix #1469

Why is this Pull Request needed?

we are currently dropping any PES packet without PTS/DTS
this was introduced by 9a80c98. this filtering was added as we were previously providing AVC samples with undefined PTS/DTS to mp4-remuxer.
the mp4-remuxer isnt resilient to that and is generating corrupted video fmp4 in that case.
let's try to be more resilient and fallback on last AVC sample PTS/DTS (PTS/DTS are optional fields of PES headers)

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

we need to test against streams with PES not signalling PTS/DTS.

Resolves issues:

#1469

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md

@stale
Copy link

stale bot commented Mar 31, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale label Mar 31, 2019
@stale stale bot closed this Apr 7, 2019
@johnBartos
Copy link
Collaborator

johnBartos commented Apr 7, 2019 via email

@johnBartos johnBartos reopened this Apr 15, 2019
@johnBartos johnBartos added this to the 1.0.0 milestone Jun 19, 2019
@robwalch robwalch added this to Top priorities in Release Planning and Backlog Oct 29, 2019
robwalch pushed a commit to jwplayer/hls.js that referenced this pull request Dec 6, 2019
Prevent RangeError exception when parsing incomplete PES
832fc64 
Fixes video-dev#2415
Relates to video-dev#2109
@robwalch robwalch mentioned this pull request Dec 6, 2019
Merged
3 tasks
@robwalch
Copy link
Collaborator

robwalch commented Dec 6, 2019

I’ve run into issue #2415 with RangError exceptions in tsdemuxer that this fixes.

I’m just a little worried about there being issues with not just AVC but AAC and ID3 also missing timestamps.

@robwalch robwalch modified the milestones: 1.0.0, 0.13.1 Dec 6, 2019
@robwalch robwalch self-requested a review December 6, 2019 19:39
robwalch
robwalch approved these changes Dec 6, 2019
View reviewed changes
Copy link
Collaborator

@robwalch robwalch left a comment

Choose a reason for hiding this comment

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

These changes prevent the RangeError exception in #2415 that we're now guarding against with #2463.

I'd like to do a little more testing before merging so bumping this to v0.13.1.

robwalch pushed a commit that referenced this pull request Dec 8, 2019
Merge branch 'upstream/master' into feature/v1.0.0
37537ff 
* upstream/master: (81 commits)
  Prevent seeking to 0 when startPosition is -1 Fixes #2438
  Prevent RangeError exception when parsing incomplete PES Fixes #2415 Relates to #2109
  Log license key size (DRM functional test troubleshooting)
  Improve functional test assertion feedback
  Handle stalls when seeking into buffer gaps
  Update gap controller to observe buffer gaps regardless of max buffer hole
  Update gap-controller comments
  Add seek back to start functional test
  Fix start gap jumping with partial fragments and when seeking back to start
  Reset stall time after seeked and fix logger statments
  Fix partial and large start gap jumping
  Do not jump gaps larger than JUMP_THRESHOLD_SECONDS
  Update gap-controller comments
  Avoid jumping gaps that Chrome will jump Prevent reporting stalls right after "seeked"
  Jump start gap in Safari fixes #2436
  Update gap-controller to match v1 branch
  gap-controller: add handling for audio-only autoplay issues in Chrome
  gap-controller: improve some log lines
  gap-controller: add unit tests for initial gap skipping + paused/hasPlayed cases
  gap-controller: documentation/comments improvements
  ...

# Conflicts:
#	package-lock.json
#	scripts/travis.sh
#	src/controller/audio-stream-controller.js
#	src/controller/audio-track-controller.ts
#	src/controller/buffer-controller.ts
#	src/controller/gap-controller.js
#	src/controller/id3-track-controller.js
#	src/controller/stream-controller.js
#	src/controller/subtitle-stream-controller.ts
#	src/controller/subtitle-track-controller.ts
#	src/controller/timeline-controller.ts
#	src/demux/tsdemuxer.ts
#	src/events.js
#	src/loader/m3u8-parser.ts
#	src/utils/buffer-helper.ts
#	tests/functional/auto/setup.js
#	tests/test-streams.js
#	tests/unit/controller/ewma-bandwidth-estimator.js
#	tests/unit/controller/gap-controller.js
#	tests/unit/controller/subtitle-stream-controller.js
@robwalch robwalch mentioned this pull request Dec 8, 2019
Merged
robwalch pushed a commit that referenced this pull request Dec 9, 2019
Merge branch 'upstream/master' into feature/v1.0.0
94bafae 
* upstream/master: (81 commits)
  Prevent seeking to 0 when startPosition is -1 Fixes #2438
  Prevent RangeError exception when parsing incomplete PES Fixes #2415 Relates to #2109
  Log license key size (DRM functional test troubleshooting)
  Improve functional test assertion feedback
  Handle stalls when seeking into buffer gaps
  Update gap controller to observe buffer gaps regardless of max buffer hole
  Update gap-controller comments
  Add seek back to start functional test
  Fix start gap jumping with partial fragments and when seeking back to start
  Reset stall time after seeked and fix logger statments
  Fix partial and large start gap jumping
  Do not jump gaps larger than JUMP_THRESHOLD_SECONDS
  Update gap-controller comments
  Avoid jumping gaps that Chrome will jump Prevent reporting stalls right after "seeked"
  Jump start gap in Safari fixes #2436
  Update gap-controller to match v1 branch
  gap-controller: add handling for audio-only autoplay issues in Chrome
  gap-controller: improve some log lines
  gap-controller: add unit tests for initial gap skipping + paused/hasPlayed cases
  gap-controller: documentation/comments improvements
  ...

# Conflicts:
#	package-lock.json
#	scripts/travis.sh
#	src/controller/audio-stream-controller.js
#	src/controller/audio-track-controller.ts
#	src/controller/buffer-controller.ts
#	src/controller/gap-controller.js
#	src/controller/id3-track-controller.js
#	src/controller/stream-controller.js
#	src/controller/subtitle-stream-controller.ts
#	src/controller/subtitle-track-controller.ts
#	src/controller/timeline-controller.ts
#	src/demux/tsdemuxer.ts
#	src/events.js
#	src/loader/m3u8-parser.ts
#	src/utils/buffer-helper.ts
#	tests/functional/auto/setup.js
#	tests/test-streams.js
#	tests/unit/controller/ewma-bandwidth-estimator.js
#	tests/unit/controller/gap-controller.js
#	tests/unit/controller/subtitle-stream-controller.js
robwalch pushed a commit that referenced this pull request Dec 9, 2019
Merge branch 'upstream/master' into feature/v1.0.0
8ad6dcc 
* upstream/master: (81 commits)
  Prevent seeking to 0 when startPosition is -1 Fixes #2438
  Prevent RangeError exception when parsing incomplete PES Fixes #2415 Relates to #2109
  Log license key size (DRM functional test troubleshooting)
  Improve functional test assertion feedback
  Handle stalls when seeking into buffer gaps
  Update gap controller to observe buffer gaps regardless of max buffer hole
  Update gap-controller comments
  Add seek back to start functional test
  Fix start gap jumping with partial fragments and when seeking back to start
  Reset stall time after seeked and fix logger statments
  Fix partial and large start gap jumping
  Do not jump gaps larger than JUMP_THRESHOLD_SECONDS
  Update gap-controller comments
  Avoid jumping gaps that Chrome will jump Prevent reporting stalls right after "seeked"
  Jump start gap in Safari fixes #2436
  Update gap-controller to match v1 branch
  gap-controller: add handling for audio-only autoplay issues in Chrome
  gap-controller: improve some log lines
  gap-controller: add unit tests for initial gap skipping + paused/hasPlayed cases
  gap-controller: documentation/comments improvements
  ...

# Conflicts:
#	package-lock.json
#	scripts/travis.sh
#	src/controller/audio-stream-controller.js
#	src/controller/audio-track-controller.ts
#	src/controller/buffer-controller.ts
#	src/controller/gap-controller.js
#	src/controller/id3-track-controller.js
#	src/controller/stream-controller.js
#	src/controller/subtitle-stream-controller.ts
#	src/controller/subtitle-track-controller.ts
#	src/controller/timeline-controller.ts
#	src/demux/tsdemuxer.ts
#	src/events.js
#	src/loader/m3u8-parser.ts
#	src/utils/buffer-helper.ts
#	tests/functional/auto/setup.js
#	tests/test-streams.js
#	tests/unit/controller/ewma-bandwidth-estimator.js
#	tests/unit/controller/gap-controller.js
#	tests/unit/controller/subtitle-stream-controller.js
@robwalch robwalch modified the milestones: 0.13.1, 0.13.2 Jan 6, 2020
robwalch pushed a commit that referenced this pull request Jan 8, 2020
Merge branch 'upstream/master' into feature/v1.0.0
06e48aa 
* upstream/master: (92 commits)
  Update package-lock
  Update TypeScript and run `npm audit fix`
  Add a lint rule to disallow use of SourceBuffer global
  Fix `isSupported` check in browser missing `SourceBuffer` global Fixes #2430 #2476
  https
  add code of conduct
  Remove build:types from release job.
  Remove TypeScript types build step
  only include js files in release
  Prevent seeking to 0 when startPosition is -1 Fixes #2438
  Prevent RangeError exception when parsing incomplete PES Fixes #2415 Relates to #2109
  Log license key size (DRM functional test troubleshooting)
  Improve functional test assertion feedback
  Handle stalls when seeking into buffer gaps
  Update gap controller to observe buffer gaps regardless of max buffer hole
  Update gap-controller comments
  Add seek back to start functional test
  Fix start gap jumping with partial fragments and when seeking back to start
  Reset stall time after seeked and fix logger statments
  Fix partial and large start gap jumping
  ...

# Conflicts:
#	package-lock.json
#	package.json
#	scripts/travis.sh
#	src/controller/audio-stream-controller.js
#	src/controller/buffer-controller.ts
#	src/controller/gap-controller.js
#	src/controller/id3-track-controller.js
#	src/controller/stream-controller.js
#	src/controller/subtitle-stream-controller.ts
#	src/controller/subtitle-track-controller.ts
#	src/controller/timeline-controller.ts
#	src/demux/tsdemuxer.ts
#	src/events.js
#	src/loader/m3u8-parser.ts
#	src/utils/buffer-helper.ts
#	tests/functional/auto/setup.js
#	tests/test-streams.js
#	tests/unit/controller/ewma-bandwidth-estimator.js
#	tests/unit/controller/gap-controller.js
#	tests/unit/controller/subtitle-stream-controller.js
robwalch pushed a commit to jwplayer/hls.js that referenced this pull request Jan 8, 2020
Merge branch 'feature/v1.0.0' into bugfix/dts-sync
9232089 
* feature/v1.0.0: (95 commits)
  Enforce no-case-declarations
  Cleanup timeout calls
  Enforce 'no-unused-vars' lint rule
  Update package-lock
  Update TypeScript and run `npm audit fix`
  Add a lint rule to disallow use of SourceBuffer global
  Fix `isSupported` check in browser missing `SourceBuffer` global Fixes video-dev#2430 video-dev#2476
  https
  add code of conduct
  Remove build:types from release job.
  Remove TypeScript types build step
  only include js files in release
  Prevent seeking to 0 when startPosition is -1 Fixes video-dev#2438
  Prevent RangeError exception when parsing incomplete PES Fixes video-dev#2415 Relates to video-dev#2109
  Log license key size (DRM functional test troubleshooting)
  Improve functional test assertion feedback
  Handle stalls when seeking into buffer gaps
  Update gap controller to observe buffer gaps regardless of max buffer hole
  Update gap-controller comments
  Add seek back to start functional test
  ...
@robwalch
Copy link
Collaborator

robwalch commented Jan 9, 2020

I've tested these changes in the feature/v1.0.0 branch and they work very nicely!

@robwalch robwalch merged commit 12f2e16 into master Jan 9, 2020
@robwalch robwalch deleted the bugfix/1469 branch January 9, 2020 19:50
robwalch pushed a commit to jwplayer/hls.js that referenced this pull request Jan 9, 2020
Merge branch 'feature/v1.0.0' into LHLS
a8f1a7e 
* feature/v1.0.0: (96 commits)
  Fix audio gaps introduced by changes in remuxer continuity tracking
  Enforce no-case-declarations
  Cleanup timeout calls
  Enforce 'no-unused-vars' lint rule
  Update package-lock
  Update TypeScript and run `npm audit fix`
  Add a lint rule to disallow use of SourceBuffer global
  Fix `isSupported` check in browser missing `SourceBuffer` global Fixes video-dev#2430 video-dev#2476
  https
  add code of conduct
  Remove build:types from release job.
  Remove TypeScript types build step
  only include js files in release
  Prevent seeking to 0 when startPosition is -1 Fixes video-dev#2438
  Prevent RangeError exception when parsing incomplete PES Fixes video-dev#2415 Relates to video-dev#2109
  Log license key size (DRM functional test troubleshooting)
  Improve functional test assertion feedback
  Handle stalls when seeking into buffer gaps
  Update gap controller to observe buffer gaps regardless of max buffer hole
  Update gap-controller comments
  ...
@snyk-bot snyk-bot mentioned this pull request Mar 8, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robwalch robwalch robwalch approved these changes

Assignees
No one assigned
Labels
Confirmed
Projects
HLS.js Release Planning and Backlog
Archived in project
Release Planning and Backlog
  
Top priorities
Milestone
0.13.2
Development

Successfully merging this pull request may close these issues.

Player stop playing and loading after second seek
3 participants
@mangui @johnBartos @robwalch