grafov / m3u8

Parser and generator of M3U8-playlists for Apple HLS. Library for Go language. :cinema:
http://tools.ietf.org/html/draft-pantos-http-live-streaming
BSD 3-Clause "New" or "Revised" License
1.23k stars 315 forks source link

Add values to VariantParams #89

Closed kaihendry closed 7 years ago

kaihendry commented 7 years ago

Sorry, I am a Golang newbie and my use case is to introduce a new key into the variant called 'rendition-id' to help clients create a rendition switcher.

package main

import (
    "fmt"

    "github.com/grafov/m3u8"
)

// https://godoc.org/github.com/grafov/m3u8#VariantParams

type myVariantParams struct {
    *m3u8.VariantParams
    RenditionId string
}

func main() {

    masterPlaylist := m3u8.NewMasterPlaylist()

    pp, _ := m3u8.NewMediaPlaylist(3, 5)
    for i := 0; i < 5; i++ {
        pp.Append(fmt.Sprintf("test%d.ts", i), 5.0, "")
    }

    // WORKS
    masterPlaylist.Append("chunklist2.m3u8", pp, m3u8.VariantParams{ProgramId: 123, Bandwidth: 1500000, Resolution: "576x480"})

    // Trying to extend VariantParams doesn't !
    // masterPlaylist.Append("chunklist2.m3u8", pp, myVariantParams{VariantParams: &m3u8.VariantParams{ProgramId: 123, Bandwidth: 1500000, Resolution: "576x480"}, RenditionId: "foo"})

    fmt.Println(len(masterPlaylist.Variants))
    masterPlaylist.ResetCache()

    fmt.Println(masterPlaylist.Encode().String())

}

Perhaps this is the same as #82. Anyway, just thought I'd convey my use case. My tact now is to fork and add the missing values.

bradleyfalzon commented 7 years ago

The error you should be getting is:

./main.go:29: cannot use myVariantParams literal (type myVariantParams) as type m3u8.VariantParams in argument to masterPlaylist.Append

This is because the append method on MasterPlaylist is

func (p *MasterPlaylist) Append(uri string, chunklist *MediaPlaylist, params VariantParams)

As Go is a strongly typed language, the only type that can be used for the 3rd argument is a type github.com/grafov/m3u8.VariantParams.

So you won't be able to do exactly what you want this way.

If the HLS RFC does have this as a field, we can add it to the VariantParams type, but if you're trying to add a custom field to be added when calling the String() method, then you'll need to do a lot more work with how the API is right now (we don't support custom fields).

Side note, because the Append method already resets the cache, you don't need to do that.

    masterPlaylist.ResetCache()

Removing the above line shouldn't change the output.

kaihendry commented 7 years ago

I ended up adding what I needed by forking the repo here: https://github.com/Spuul/go-m3u8/commit/a418099cf75c3a917a309d5cd1d3641c6fdeee84

bradleyfalzon commented 7 years ago

Yeah that would be the change that's required to add support for that field (to write that field, but you'll need to do more to read the field), I take it this isn't a field in the HLS RFC? If it is, we'll be happy to add that field ourselves.

kaihendry commented 7 years ago

It's a custom field. We're also using a Javascript library https://github.com/tedconf/node-m3u8 which has a non-annoying item.set API. I am not sure how that works in a strongly typed sense..

bradleyfalzon commented 7 years ago

If it's a custom field and you're OK with running a fork, then that'll be the best option until we support some kind of custom fields, such as VariantParams taking on a X map[string]string type.

grafov commented 7 years ago

There was another request for custom fields: #82. I think we need some time for clarifying needs for this and implementing it without breaking existent API. So thank for a question!

kaihendry commented 7 years ago

Another field I need is AVERAGE-BANDWIDTH.. guess I will be adding it to my fork. https://github.com/Spuul/go-m3u8

bradleyfalzon commented 7 years ago

Any field in the the HLS draft RFC can be added to this library by either asking if we can implement or sending a pull request. It's only custom fields that there's a limitation and a fork may be your best option.

kaihendry commented 7 years ago

Ok, I'll try implement it and do a pull request spefically only upon AVERAGE-BANDWIDTH