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

Question: #EXT-X-MEDIA:TYPE=AUDIO #96

Open
md2k opened this issue Sep 19, 2017 · 11 comments · May be fixed by #158
Open

Question: #EXT-X-MEDIA:TYPE=AUDIO #96

md2k opened this issue Sep 19, 2017 · 11 comments · May be fixed by #158

Comments

@md2k
Copy link
Contributor

md2k commented Sep 19, 2017

Looks like i can get information about alternatives only from first element from variants array.
Playlist example:

#EXTM3U
#EXT-X-VERSION:5
#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="audio",NAME="Audio 0",DEFAULT=YES,AUTOSELECT=YES,LANGUAGE="ger",URI="audio_3/playlist.m3u8"
#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="audio",NAME="Audio 1",DEFAULT=NO,AUTOSELECT=YES,LANGUAGE="ger",URI="audio_4/playlist.m3u8"
#EXT-X-STREAM-INF:PROGRAM-ID=0,BANDWIDTH=997142,CODECS="avc1.4d0029",AUDIO="audio"
track_0_800/playlist.m3u8
#EXT-X-STREAM-INF:PROGRAM-ID=0,BANDWIDTH=1880000,CODECS="avc1.4d0029",AUDIO="audio"
track_1_1600/playlist.m3u8
#EXT-X-STREAM-INF:PROGRAM-ID=0,BANDWIDTH=2736607,CODECS="avc1.4d0029",AUDIO="audio"
track_2_2500/playlist.m3u8

Dumped Variants from masterpl := p.(*m3u8.MasterPlaylist)

{
    "Variants": [
        {
            "URI": "track_0_800/playlist.m3u8",
            "Chunklist": null,
            "ProgramId": 0,
            "Bandwidth": 997142,
            "Codecs": "avc1.4d0029",
            "Resolution": "",
            "Audio": "audio",
            "Video": "",
            "Subtitles": "",
            "Captions": "",
            "Name": "",
            "Iframe": false,
            "Alternatives": [
                {
                    "GroupId": "audio",
                    "URI": "audio_3/playlist.m3u8",
                    "Type": "AUDIO",
                    "Language": "ger",
                    "Name": "Audio 0",
                    "Default": true,
                    "Autoselect": "YES",
                    "Forced": "",
                    "Characteristics": "",
                    "Subtitles": ""
                },
                {
                    "GroupId": "audio",
                    "URI": "audio_4/playlist.m3u8",
                    "Type": "AUDIO",
                    "Language": "ger",
                    "Name": "Audio 1",
                    "Default": false,
                    "Autoselect": "YES",
                    "Forced": "",
                    "Characteristics": "",
                    "Subtitles": ""
                }
            ]
        },
        {
            "URI": "track_1_1600/playlist.m3u8",
            "Chunklist": null,
            "ProgramId": 0,
            "Bandwidth": 1880000,
            "Codecs": "avc1.4d0029",
            "Resolution": "",
            "Audio": "audio",
            "Video": "",
            "Subtitles": "",
            "Captions": "",
            "Name": "",
            "Iframe": false,
            "Alternatives": null
        },
        {
            "URI": "track_2_2500/playlist.m3u8",
            "Chunklist": null,
            "ProgramId": 0,
            "Bandwidth": 2736607,
            "Codecs": "avc1.4d0029",
            "Resolution": "",
            "Audio": "audio",
            "Video": "",
            "Subtitles": "",
            "Captions": "",
            "Name": "",
            "Iframe": false,
            "Alternatives": null
        }
    ],
    "Args": "",
    "CypherVersion": ""
}

This is correct behaviour, shouldn't we have information about alternatives for each variant?

@bradleyfalzon
Copy link
Collaborator

bradleyfalzon commented Sep 19, 2017

Yes, I agree this is a bug, EXT-X-MEDIA is referenced by its GROUP-ID tag, not the position in the playlist (i.e. it doesn't behave like EXT-X-STREAM-INF does when affecting the URI).

Did you want to take a stab at fixing this? It might require a bit more post processing to keep a list of all EXT-X-MEDIA and then populate each of the MasterPlaylist with the valid alternatives by matching the types and IDs correctly. I.e. in this case each variant defined AUDIO="audio", so the alternatives should have been all EXT-X-MEDIA tags where TYPE=AUDIO and ID="audio". If I'm understanding this correctly.

Relevant parts of the RFC:

https://tools.ietf.org/html/rfc8216#section-4.3.4.1:

   The EXT-X-MEDIA tag is used to relate Media Playlists that contain
   alternative Renditions (Section 4.3.4.2.1) of the same content.  For
   example, three EXT-X-MEDIA tags can be used to identify audio-only
   Media Playlists that contain English, French, and Spanish Renditions
   of the same presentation.  Or, two EXT-X-MEDIA tags can be used to
   identify video-only Media Playlists that show two different camera
   angles.

....

      GROUP-ID

      The value is a quoted-string that specifies the group to which the
      Rendition belongs.  See Section 4.3.4.1.1.  This attribute is
      REQUIRED.

https://tools.ietf.org/html/rfc8216#section-4.3.4.2:

      AUDIO

      The value is a quoted-string.  It MUST match the value of the
      GROUP-ID attribute of an EXT-X-MEDIA tag elsewhere in the Master
      Playlist whose TYPE attribute is AUDIO.  It indicates the set of
      audio Renditions that SHOULD be used when playing the
      presentation.  See Section 4.3.4.2.1.

Two good examples: https://tools.ietf.org/html/rfc8216#section-8.6 and https://tools.ietf.org/html/rfc8216#section-8.7

@md2k
Copy link
Contributor Author

md2k commented Sep 20, 2017

hi @bradleyfalzon probably i can take this bug and fix it, but currently pretty busy, so i think some more or less good output will be somewhere end of next week or week after.
I'll write later here how i see alternatives inside structure.

@md2k
Copy link
Contributor Author

md2k commented Sep 20, 2017

we probably can keep as additional all alternatives mapped by group id separately, and add methods to Variant to get correct alternatives if Variant Audio match any Variant Group-ID?

@bradleyfalzon
Copy link
Collaborator

hi @bradleyfalzon probably i can take this bug and fix it, but currently pretty busy, so i think some more or less good output will be somewhere end of next week or week after.

That sounds great, if you start running out of time or it's getting hard, let me know.

we probably can keep as additional all alternatives mapped by group id separately, and add methods to Variant to get correct alternatives if Variant Audio match any Variant Group-ID?

That's how I'd probably solve it given if this was adding the functionality, but because we're trying to maintain backwards compatibility, we'd first need to correctly populate those Alternative structs as the RFC has outlined. We could then add those other methods later and even deprecate the Alternative slice.

For a backwards compatibility point of view, we could (not a complete set of tasks):

  1. Modify the decodeState.Alternatives to be a map[string][]Alternative where string is the Group ID.
  2. On decodeLineOfMasterPlaylist append to these alternatives via state.Alternative[alt.Type] = append(state.Alternative[alt.Type], alt) (or along those lines
  3. Before MasterPlaylist.decode returns, loop over each p.Variant checking if Audio (or Video etc) has a matching decodeState.Alternatives[string] and appending to p.Variant.Alternatives

This is backwards compatible, but we could add methods as you've suggested, something like func (v *Variant) Alternatives(type string) []Alternatives.

Does that sound possible?

@md2k
Copy link
Contributor Author

md2k commented Sep 20, 2017

yea, i think this way we go.

@md2k
Copy link
Contributor Author

md2k commented Sep 28, 2017

Found one more bug or "feature" :D
in case if manifest include 2 audio TAGs with same parameters except URL, parser picks only one of them.
example of such manifest

#EXTM3U
#EXT-X-VERSION:3
#EXT-X-INDEPENDENT-SEGMENTS
#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="program_audio",LANGUAGE="eng",NAME="audio_1",AUTOSELECT=YES,DEFAULT=YES
#EXT-X-STREAM-INF:BANDWIDTH=2890800,AVERAGE-BANDWIDTH=2890800,CODECS="avc1.640029,mp4a.40.2",RESOLUTION=1280x720,FRAME-RATE=24.000,AUDIO="program_audio"
index2500.m3u8
#EXT-X-STREAM-INF:BANDWIDTH=1900800,AVERAGE-BANDWIDTH=1900800,CODECS="avc1.640029,mp4a.40.2",RESOLUTION=854x480,FRAME-RATE=24.000,AUDIO="program_audio"
index1600.m3u8
#EXT-X-STREAM-INF:BANDWIDTH=1020800,AVERAGE-BANDWIDTH=1020800,CODECS="avc1.4d401f,mp4a.40.2",RESOLUTION=640x360,FRAME-RATE=24.000,AUDIO="program_audio"
index800.m3u8
#EXT-X-STREAM-INF:BANDWIDTH=580800,AVERAGE-BANDWIDTH=580800,CODECS="avc1.42c01f,mp4a.40.2",RESOLUTION=428x240,FRAME-RATE=24.000,AUDIO="program_audio"
index400.m3u8
#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="program_audio",LANGUAGE="eng",NAME="audio_1",AUTOSELECT=YES,DEFAULT=YES,URI="indexaudio.m3u8"

While i'm agreed that this is not a bit normal, but this i got from Akamai multiple times. so i will combine probably initial ticket with this one together or should i create another ticket to cover this problem?

@bradleyfalzon
Copy link
Collaborator

in case if manifest include 2 audio TAGs with same parameters except URL, parser picks only one of them.

Appears to have been added in e045850, but the commit doesn't appear to indicate why.

https://tools.ietf.org/html/rfc8216#section-4.3.4.2.1 says:

   The URI attribute of the EXT-X-MEDIA tag is REQUIRED if the media
   type is SUBTITLES, but OPTIONAL if the media type is VIDEO or AUDIO.
   If the media type is VIDEO or AUDIO, a missing URI attribute
   indicates that the media data for this Rendition is included in the
   Media Playlist of any EXT-X-STREAM-INF tag referencing this EXT-
   X-MEDIA tag.  If the media TYPE is AUDIO and the URI attribute is
   missing, clients MUST assume that the audio data for this Rendition
   is present in every video Rendition specified by the EXT-X-STREAM-INF
   tag.

So the playlist appears to be perfectly valid, and there's definitely a bug in this library.

@grafov do you know anything about this commit and why this behaviour might be occurring? At first sight, it looks like we should revert the commit, I can't see why we'd deduplicate it OR if we do want to keep it, let's include the URI in the algorithm:

m3u8/writer.go

Line 88 in d137fcd

altKey := fmt.Sprintf("%s-%s-%s-%s", alt.Type, alt.GroupId, alt.Name, alt.Language)

@grafov
Copy link
Owner

grafov commented Sep 30, 2017

@bradleyfalzon I didn't remember details about this commit. If you found that it is breaks the logic described in the standard this commit should be reverted.
I only propose use strings.Join() instead of fmt.Sprintf() because of all args are strings.

@md2k
Copy link
Contributor Author

md2k commented Oct 12, 2017

Pardon for been quite for while (insane amount of work had), will have time next week to work over this issue.

@md2k
Copy link
Contributor Author

md2k commented Oct 31, 2017

Hi, i have problem with time at this moment, probably we'll put this issue on-hold for a bit and as soon ill have time, i'll take a look into this since i need this functionality at the end.

@bradleyfalzon
Copy link
Collaborator

No problems, we're all in similar positions.

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 a pull request may close this issue.

3 participants