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

Add values to VariantParams #89

Closed
kaihendry opened this issue Jun 2, 2017 · 9 comments
Closed

Add values to VariantParams #89

kaihendry opened this issue Jun 2, 2017 · 9 comments
Labels

Comments

@kaihendry
Copy link

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
Copy link
Collaborator

bradleyfalzon commented Jun 3, 2017

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
Copy link
Author

I ended up adding what I needed by forking the repo here: Spuul@a418099

@bradleyfalzon
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Owner

grafov commented Jun 3, 2017

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!

@grafov grafov closed this as completed Jun 3, 2017
@kaihendry
Copy link
Author

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

@bradleyfalzon
Copy link
Collaborator

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
Copy link
Author

kaihendry commented Jun 12, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants