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

Add support for sidx segments #207

Closed
wants to merge 1 commit into from

Conversation

@omarroth
Copy link

commented Aug 25, 2018

Description

Closes #162.
Requires videojs/mux.js#218

This is a sloppy first pass at adding support for SIDX segments. For some background:

A DASH representation can have a SegmentBase which looks like this:

<BaseURL>German_Forest_Short_Poem_german-de-68s-2-lc-128000bps_seg.mp4</
<SegmentBase indexRange="606-805">
    <Initialization range="0-605"/>
</SegmentBase>

The range refers to a initialization segment that contains information about the media (codec, framerate, etc). The indexRange refers to a SIDX segment which contains byte range indexes for the other segments within the file. Using videojs/mpd-parser, the above example generates a single segment with byterange: {length: 200, offset 606} and a duration equal to the total duration of the file.

http-streaming currently will immediately end given the above manifest, as it has no way of adding other segments.

SIDX is specified in ISO/IEC 14496-12.

I am using this manifest.

Specific Changes proposed

I expect this need to be refactored to fit better within http-streaming. Currently, it:

  • Looks to see if a segment is a sidx box (has header sidx)
  • If it does, set the duration of the sidx segment to 0 and parse it to get a list of references (byteranges)
  • Add new segments to the current playlist with the given byteranges

Still needs tests.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
  • Reviewed by Two Core Contributors
@welcome

This comment has been minimized.

Copy link

commented Aug 25, 2018

💖 Thanks for opening this pull request! 💖

Things that will help get your PR across the finish line:

  • Run npm run lint -- --errors locally to catch formatting errors earlier.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@ldayananda ldayananda self-assigned this Nov 2, 2018
@ldayananda

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

Thanks for the PR @omarroth! This is a great starting point for fixing #162.

I took a look at what's happening, and there's two problems:

  1. the sidx isn't exposed from mpd-parser in a way that VHS can access
  2. we need to have VHS request the sidx, parse it with mux.js and then update the playlist with the actual media segments

Doing the 2nd is non-trivial with the structure of the project, so I wound up starting on an alternative set of PRs. For 1, you can follow videojs/mpd-parser#41 and I'll post a WIP PR for the VHS portion of 2 too. Also, thanks for the mux.js PR!

@omarroth

This comment has been minimized.

Copy link
Author

commented Nov 2, 2018

I expected as such regarding 2, I seem to remember it being mentioned in another issue as well. I appreciate you taking a look!

@ldayananda

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

Any chance you can provide another manifest to develop against? Looks like the content is unavailable now for the url you provided in the issue and this PR

@omarroth

This comment has been minimized.

Copy link
Author

commented Dec 3, 2018

Closing in favor of #303.

@omarroth omarroth closed this Dec 3, 2018
@omarroth omarroth deleted the omarroth:add-sidx branch Dec 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.