search

shaka-project / shaka-packager

A media packaging and development framework for VOD and Live DASH and HLS applications, supporting Common Encryption for Widevine and other DRM Systems.
https://shaka-project.github.io/shaka-packager/
Other
1.96k stars 505 forks source link

feat: order streams in manifest based on command-line order #1329

Closed SteveR-PMP closed 7 months ago

SteveR-PMP commented 8 months ago

This will force the muxer to order streams in the order given on the command-line.

Closes #560 Closes #1280 Closes #1313

joeyparrish commented 7 months ago

@SteveR-PMP, if you are okay with the two edits I made, and if the builds and tests all pass, I will merge this for release in v3.0.0.

vish91 commented 7 months ago

@joeyparrish @SteveR-PMP one comment would be to change hls_params and other references initializing bool to true so that's default when the cli argument is not passed ?
Also documentation change to explicitly note that this is the default behavior and set it to false to get old random ordering behavior

joeyparrish commented 7 months ago

Looks like test outputs may need to be updated.

cosmin commented 7 months ago

There may also be some bugs here, I'm looking at the test output and seeing some cases where adaptation set ID ends up being 4294967295 which might mean there are some code paths that are not setting this properly. I'm digging into it.

joeyparrish commented 7 months ago

Thank you, @cosmin. I would welcome any changes you want to push to the fork for this PR to correct these issues before we merge it.

cosmin commented 7 months ago

I'm taking a look to see if there's a way to do this more cleanly.

cosmin commented 7 months ago

@joeyparrish take a look at the changes I pushed and see if it still looks good to you

SteveR-PMP commented 7 months ago

@cosmin The changes look good to me, except can we stay with the variable name of 'cl_index' since it is associated with the command-line option 'force_cl_index'? 'index' seems too generic of a name.

cosmin commented 7 months ago

@SteveR-PMP it should be generic, the underlying code may not be called from a command line and yet an index can be provided. The flag (on by default) is whether the command line interface will set the order, and nothing outside of the packager_main ought to look at the force_cl_index flag.

cosmin commented 7 months ago

It builds locally, this usually means there's some missing include of probably optional somewhere, I'll track it down.