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

feat: Transmux Before Append and LHLS #272

Closed
wants to merge 253 commits into from

Conversation

gesinger
Copy link
Contributor

@gesinger gesinger commented Oct 25, 2018

Includes work by: @mjneil @ldayananda @gesinger

This PR refactors our segment handling logic by transmuxing a segment before appending it to the source buffer, and adds support for LHLS (low latency HLS) in the process.

This allows us to remove the legacy videojs-contrib-media-sources code where we faked the source buffers to allow us to append TS segments directly (and transmuxed within the fake source buffer). There are many reasons we chose to do this, and for a more detailed look into why we went this route, please see the docs dir in this branch: https://github.com/gesinger/http-streaming/blob/transmux-before-append-lhls/docs/lhls/index.md .

Most of the changes are in test files and package-lock.json, however, there is a good deal of refactoring throughout a large part of the codebase. In addition, some legacy code was cleaned up, and some bug fixes were made.

Portions Reviewed:

Due to the size of this PR, it may make the most sense to review in portions. From individual modules to logical paths.

Docs, linting, and scripts

  • eslint changes (extending videojs' standard, overriding for tests)
  • docs/creating-content.md - sample ffmpeg commands for creating (and created) content
  • docs/lhls/index.md - overview of transmux before append and LHLS work
  • docs/lhls/transmux-before-append-changes.md
  • scripts/segments-data.js - script changes for real segments in tests
  • scripts/test.rollup.config.js - script changes for async/await in tests

Modules

and associated tests

  • src/master-playlist-controller.js
  • src/media-groups.js
  • src/media-segment-request.js
  • src/playback-watcher.js
  • src/segment-loader.js
  • src/segment-transmuxer.js
  • src/source-updater.js
  • src/sync-controller.js
  • src/transmuxer-worker.js
  • src/util/codecs.js
  • src/util/segment.js
  • src/util/string-to-array-buffer.js
  • src/util/text-tracks.js
  • src/videojs-http-streaming.js
  • src/vtt-segment-loader.js

Logic Paths and Areas of Focus

and associated tests

  • (real) media source and source buffer creation (largely in master-playlist-controller)
  • duration updates for live and VOD (largely in master-playlist-controller)
  • handling partial data (largely in media-segment-request) (including captions and ID3 tags)
  • removed code from videojs-contrib-media-sources
  • captions (largely in segment-loader)
  • id3 metadata (largely in segment-loader)
  • WebWorker creation, management, and cleanup (largely in segment-loader)
  • discontinuities/timestamp offsets (largely in segment-loader)
  • gopsToAlignWith
  • segment-loader queueing
  • segment start and end times (largely in segment-loader)
  • source-updater queueing

Known Issues

  • Alt audio with handlePartialData set to true
  • Alt audio, when offset from a 0 start time, sometimes causes video to forever buffer. This happens when the audio downloads and appends before the video has time to set the timestamp offset on the source buffer (therefore, audio will remain offset like the content itself rather than start at 0). Keeping in mind discontinuities/timelines, the audio segment loader will need to wait for video to update the source buffer's timestamp offset (if necessary) within a timeline before appending its own content.
  • Muxed fmp4 content (HLS) that doesn't provide codec info in the master manifest doesn't play. This is due to us not parsing the fmp4 segments, and assuming that content is demuxed if using fmp4. We will need to parse the fmp4 segments and not assume demuxed in order to resolve this issue.
  • Doubled captions when entering fullscreen. This also happens when switching renditions. It appears the caption timestamps we get back from mux.js are different for different renditions(but the same caption text). It just so happens when entering fullscreen, because we use keepOriginalTimestamps: true for this branch, the caption timestamps align and are shown at the same time. Both on master and when doing rendition switches, the captions are actually added twice, but with very different timestamps so they are not displayed together. It remains to be investigated why we are getting these incorrect timestamps back from mux.js in the first place.

Explicit Areas to Test

  • Not appending video init segment for every segment. From @mjneil: "I think we tried this in the past, but had to leave it as appending video init for every segment cause of a browser issue."

gesinger and others added 30 commits March 14, 2018 17:15
Copy link
Contributor

@forbesjo forbesjo left a comment

Choose a reason for hiding this comment

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

fmp4 content doesn't seem to be working (https://d2zihajmogu5jn.cloudfront.net/advanced-fmp4/master.m3u8), it doesn't look like any segments are being appended

This issue is noted in the Known Issues section of the PR

There also seems to be a similar issue with DASH DRM content

…one append finishes (waits for all queued actions)
@gkatsev
Copy link
Member

gkatsev commented Mar 22, 2019

There's work to resolve conflicts in a PR but it requires a bit more work than just merging that PR, so, we should use it as inspiration. We also have plans to slowly work through this and review and then merge and fix forward any issues.

@gesinger gesinger mentioned this pull request Apr 11, 2019
34 tasks
@gkatsev
Copy link
Member

gkatsev commented Apr 11, 2019

further work will be happening in #466

@gkatsev gkatsev closed this Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants