fix: exact-order-hls

[SteveR-PMP]

HLS manifest was still grouping by audio codecs first in the manifest before forcing the command line order.

[vish91]

@joeyparrish @cosmin if you can help review this please. We found a bug with the input command based ordering feature where it was working correctly for DASH but not for HLS because of this grouping logic baked into packager only in HLS. so this rework from @SteveR-PMP should help fix that and make that feature a true command input order for both HLS and DASH.

[SteveR-PMP]

as an example, the following command line:

packager \ in=audio.mp4,stream=audio,hls_group_id=ec3,output=audio2.mp4 \ in=audio.mp4,stream=audio,hls_group_id=aac,output=audio3.mp4 \ in=audio.mp4,stream=audio,hls_group_id=ec3,output=audio4.mp4 \ in=audio.mp4,stream=audio,hls_group_id=aac,output=audio5.mp4 \ in=video.mp4,stream=video,output=video2.mp4 \ --hls_master_playlist_output master.m3u8

would previously generate a HLS manifest that grouped by hls_group_id first:

#EXT-X-MEDIA:TYPE=AUDIO,URI="stream_0.m3u8",GROUP-ID="ec3",NAME="stream_0",DEFAULT=NO,AUTOSELECT=YES,CHANNELS="2"
#EXT-X-MEDIA:TYPE=AUDIO,URI="stream_2.m3u8",GROUP-ID="ec3",NAME="stream_2",DEFAULT=NO,CHANNELS="2"
#EXT-X-MEDIA:TYPE=AUDIO,URI="stream_1.m3u8",GROUP-ID="aac",NAME="stream_1",DEFAULT=NO,AUTOSELECT=YES,CHANNELS="2"
#EXT-X-MEDIA:TYPE=AUDIO,URI="stream_3.m3u8",GROUP-ID="aac",NAME="stream_3",DEFAULT=NO,CHANNELS="2"

#EXT-X-STREAM-INF:BANDWIDTH=2354148,AVERAGE-BANDWIDTH=880371,CODECS="avc1.64001e,mp4a.40.2",RESOLUTION=720x300,FRAME-RATE=24.000,AUDIO="aac",CLOSED-CAPTIONS=NONE
stream_4.m3u8

#EXT-X-STREAM-INF:BANDWIDTH=2354148,AVERAGE-BANDWIDTH=880371,CODECS="avc1.64001e,mp4a.40.2",RESOLUTION=720x300,FRAME-RATE=24.000,AUDIO="ec3",CLOSED-CAPTIONS=NONE
stream_4.m3u8

and now it will force the exact command-line order:

#EXT-X-MEDIA:TYPE=AUDIO,URI="stream_0.m3u8",GROUP-ID="ec3",NAME="stream_0",DEFAULT=NO,AUTOSELECT=YES,CHANNELS="2"
#EXT-X-MEDIA:TYPE=AUDIO,URI="stream_1.m3u8",GROUP-ID="aac",NAME="stream_1",DEFAULT=NO,AUTOSELECT=YES,CHANNELS="2"
#EXT-X-MEDIA:TYPE=AUDIO,URI="stream_2.m3u8",GROUP-ID="ec3",NAME="stream_2",DEFAULT=NO,CHANNELS="2"
#EXT-X-MEDIA:TYPE=AUDIO,URI="stream_3.m3u8",GROUP-ID="aac",NAME="stream_3",DEFAULT=NO,CHANNELS="2"

#EXT-X-STREAM-INF:BANDWIDTH=2354148,AVERAGE-BANDWIDTH=880371,CODECS="avc1.64001e,mp4a.40.2",RESOLUTION=720x300,FRAME-RATE=24.000,AUDIO="aac",CLOSED-CAPTIONS=NONE
stream_4.m3u8

#EXT-X-STREAM-INF:BANDWIDTH=2354148,AVERAGE-BANDWIDTH=880371,CODECS="avc1.64001e,mp4a.40.2",RESOLUTION=720x300,FRAME-RATE=24.000,AUDIO="ec3",CLOSED-CAPTIONS=NONE
stream_4.m3u8

[cosmin]

And the groups no longer being grouped together is what we want? On the one hand I guess it's not really strict command line ordering if we preserve the groups being grouped, on the other hand it's not clear where that would matter, as long as we respect the ordering within each group.

[cosmin]

Something about the name MediaTagslist is too visually close to MediaPlaylist to the point where I had to read the code closely several times to make sure I was reading it correctly. Perhaps we can find a better name.

[cosmin]

This seems a bit confusing, before this change BuildMediaTags called BuildMediaTag within, but now it no longer does that. We probably need to rename this function.

[cosmin]

Or alternatively keep this function as is, giving it a list of of MediaTagslist or whatever we rename that to, and instead build the right struct (with the group name, etc) in the main loop as suggested in another comment

[cosmin]

above we group playlists into group using audio_playlist_groups[GetGroupId(*playlist)].push_back(playlist); so that the groups can be sorted, but now we're no longer doing that, instead flattening the group and sorting that. We should probably insert the right structs into a list in that first for (const MediaPlaylist* playlist : playlists) { loop then we can sort each list, and no longer keep things grouped.

[SteveR-PMP]

Are you suggesting removing std::map<std::string, std::list<const MediaPlaylist*>> audio_playlist_groups and replacing it with a new std::list<MediaTagslist> audio_playlists? The audio_playlist_groups is still used by BuildVariants(), so that and BuildMediaTags() would need to be refactored.

I initially started down that route but felt that the code in BuildMediaTags() was easier to follow when grouped first. I could re-address this if you think otherwise.

[cosmin]

@SteveR-PMP ah yes, BuildVariants() still operates on groups. It still feels that using std::list<MediaTagslist> audio_playlists, refactoring BuildMediaTags() to use this (and call BuildMediaTag() within as before) and deferring any necessary grouping to BuildVariants() would be cleaner.

[vish91]

do we want to do this refactor now @cosmin ? me or @SteveR-PMP haven't really given this a try yet and test again the combinations.

[cosmin]

@vish91 I do think this needs a refactoring, one way or another.

[SteveR-PMP]

And the groups no longer being grouped together is what we want? On the one hand I guess it's not really strict command line ordering if we preserve the groups being grouped, on the other hand it's not clear where that would matter, as long as we respect the ordering within each group.

@cosmin
This is important because:

• It helps address some problems for certain players where if we do not follow command line order a non-native primary language or an AD audio track shows up first based on current grouping and leads to playback audio track selection issues.
• It should match the functionality of DASH and it avoids the confusion as to why is doesn't match the command line exactly.

If someone still wants it grouped by audio-groups first, they could manually enforce that on the command line.