m3u8-rs icon indicating copy to clipboard operation
m3u8-rs copied to clipboard

alternatives does not contain audio playlist after upgrading from 3.0.0 to 5.0.3

Open bes opened this issue 2 years ago • 16 comments

Hi, I recently upgraded from 3.0.0 to 5.0.3 and now my audio playlist ends up in unknown_tags instead of in alternatives.

What am I doing wrong?

Here is the master playlist, slightly anonymized

#EXTM3U
#EXT-X-VERSION:6
#EXT-X-INDEPENDENT-SEGMENTS
#EXT-X-MEDIA:LANGUAGE="en",AUTOSELECT=YES,FORCED=NO,TYPE=AUDIO,URI="https://xyz.com/a/playlist.m3u8",GROUP-ID="audio",DEFAULT=YES,NAME="Audio"
#EXT-X-STREAM-INF:RESOLUTION=3840x2160,FRAME-RATE=25.0,BANDWIDTH=6878544,AUDIO="audio"
https://xyz.com/b/playlist.m3u8

bes avatar Dec 15 '22 22:12 bes

The FORCED attribute is only allowed in TYPE=SUBTITLES alternatives, that's why parsing it fails.

 FORCED

 The value is an enumerated-string; valid strings are YES and NO.
 This attribute is OPTIONAL.  Its absence indicates an implicit
 value of NO.  The FORCED attribute MUST NOT be present unless the
 TYPE is SUBTITLES.

sdroege avatar Dec 16 '22 07:12 sdroege

Thank you for your help.

Is there no room for lenient parsing? What if new attributes are added to the HLS spec, will those playlists be rejected too, even if they could be played?

I would be happy if m3u8-rs could follow the robustness principle - be conservative in what you send, be liberal in what you accept.

bes avatar Dec 16 '22 07:12 bes

New attributes would be stored separately IIRC, that wouldn't be a problem.

The spec is very clear about this part (MUST NOT), that's why this is currently considered an actual parsing error. Maybe @vagetman or @rutgersc have other opinions here?

sdroege avatar Dec 16 '22 08:12 sdroege

Happy new year! Any thoughts on this issue @vagetman @rutgersc ? Thanks 🙏

bes avatar Jan 09 '23 08:01 bes

I am stuck on v3.0.0 until this issue is resolved. Unfortunately I just discovered that v3.0.0 has a bug that makes it hang forever if the m3u8_rs::parse_playlist function gets an empty slice.

Since this issue isn't making any progress, and since you're not patching v3.0.0 I am wondering if anyone can give me advice on what I should do with "bad" playlists that contain the FORCED attribute (or any other attribute) where it shouldn't be?

Should I do a first pass over the playlist and remove things myself? Seems brittle and error-prone :(

(Unfortunately my software receives bad playlists, and there's not much I can do about that)

EDIT: v4.0.0 also works for me, and doesn't have the hanging bug.

bes avatar Jan 19 '23 22:01 bes

@vagetman @rutgersc We could also add a "strict" flag to the parser and only when that is set to true it would fail parsing such things, otherwise just ignore as much as possible.

sdroege avatar Jan 20 '23 08:01 sdroege

Yeah, a parameter like that would probably be the way to go. I'm honestly kind of surprised the spec is this strict about some property belonging to a specific type.

rutgersc avatar Jan 22 '23 19:01 rutgersc

Ok, let's go with that then. I'll look into adding this some time this week.

I'm honestly kind of surprised the spec is this strict

... and on the other hand the HLS spec is extremely loose about things that actually matter. It's one of the worst specs I ever worked with.

sdroege avatar Jan 23 '23 08:01 sdroege

Sorry, I was a bit busy. @bes, to clarify - what would be the desired behavior? Did you want to ignore FORCE on the wrong alternatives during ingestion?

vagetman avatar Feb 10 '23 00:02 vagetman

@vagetman Hi! My thinking is that there are a lot of playlists out there that are plain wrong and it might be any attribute, not just FORCED.

My dream would be if m3u8-rs could parse as liniently as possible, for any attributes and any items, not just limited to FORCED.

But when it came to producing playlists itself, it would be according to spec without the possibility to introduce errors.

But how far you take it is obviously your decision.

Basically as I wrote in a message above: follow the robustness principle - be conservative in what you do, be liberal in what you accept from others

Thank you 🙏

bes avatar Feb 10 '23 06:02 bes

@vagetman Please feel free to take over here. I'm extremely busy with work-related tasks currently and this keeps getting behind on my todo list. Sorry for that.

parse as liniently as possible

FWIW this is exactly the reason why we're in this situation now. Because the parsers are very forgiving, most of the HLS streams are complete garbage and hard to handle. It's the same with various other formats.

At this point it's the only valid approach for HLS though, that ship has sailed. And additional attributes that have no valid meaning is the least of the problems in that regard :)

sdroege avatar Feb 10 '23 08:02 sdroege

Thanks, both. I'll poke around. I'm busy with Super Bowl prep and delivery this week and the weekend, but the following week should be easier.

I have a different experience with playlists though. I actually didn't see any HLS deviations around here.. But that could be because I'm usually working with big broadcasters and streamers and their playlists come from places like Elementals or Wowza. Further, customers normally want to validate their streams with Safari and QuickTime and those are not exactly forgiving ones. They simply stop playing if something is wrong without much of verbosity. I'm not sure what happens with extra attributes, they could be ignored.

Also, generally, because something is working despite the standard it's not a good example to follow. BUT I was advocating on another ticket we should support unknown attributes to support experiments or future standard revisions without having a necessity to update the library.

vagetman avatar Feb 10 '23 16:02 vagetman

@bes so, where does it actually break for you? I tried ingesting the manifest with this and it reads and prints it just fine:

    let playlist = match parsed {
        Result::Ok((_i, playlist)) => playlist,
        Result::Err(e) => panic!("Parsing error: \n{}", e),
    };

    match playlist {
        Playlist::MasterPlaylist(pl) => println!("Master playlist:\n{:?}", pl),
        Playlist::MediaPlaylist(pl) => println!("Media playlist:\n{:?}", pl),
    }

I can see some checks on FORCED in AlternativeMedia::from_hashmap(). Is this a problem? I thought the claim is that manifest parsing doesn't work.

vagetman avatar Feb 12 '23 16:02 vagetman

My original claim is that an audio playlist that has FORCED=NO on it ends up in unknown_tags which makes it much harder to work with.

bes avatar Feb 12 '23 16:02 bes

Ah, thank you @bes

vagetman avatar Feb 12 '23 16:02 vagetman

Hello everyone, I was wondering if any progress was made on this issue? Thanks 🤗

bes avatar Jul 13 '23 10:07 bes