Add availabilityStartTime attribute support

[leandromoreira]

• Changed the NewMPD constructor to accept a map of attributes (easier to expand to new attributes)
• Added a new helper function EmptyStrPtr to work with an empty string which I need to be seen as nil pointers.
• Added a new test case for availabilityStartTime
• Fixed existent tests

[leandromoreira]

What do you think of these changes?

[stuarthicks]

How about exporting these supported attributes as constants? eg AttrAvailabilityStartTime.

[stuarthicks]

Apologies for taking a while to respond to this. I've been discussing this change with others members of the team.

I like the new constructor signature from the perspective of it's shorter and neater, however there is a drawback to the map[string]string approach (aside from it being a breaking API change) in that the attributes that are available are now less discoverable (no IDE will be able to know which strings should go here). Another consideration is that the constructor signature served to declare which fields are mandatory. Using an attribute map also loses this hinting.

How about we make this two constructors? We could leave the previous constructor as-is, and add a new one NewDynamicMPD that requires and sets the availabilityStartTime attribute? This way we can avoid making a breaking API change, and we can keep using a non-map method signature to make the required parameters more obvious.

(At some point in the future, we could consider other ideas such as using a builder-pattern for mpd construction)

What do you think?

[leandromoreira]

Hi @stuarthicks no problems about the delay, we're all usually too busy =)

I liked your suggestion, I'll treat it as our first suggestion:

1. a new constructor

func NewDynamicMPD(profile DashProfile, mediaPresentationDuration string, minBufferTime string, availabilityStartTime string) *MPD {
}

2. how about making this new constructor to receive the MPD struct and we can even exclude the profile argument and retrieve all the others attributes from it and if we want to add a new attribute we would add it first to the struct and treat internally.
   A legit case could be to create an in-memory MPD struct to dump as an XML MPD.

func NewDynamicMPD(mpd *MPD) *MPD {
}

Thanks for all the input but besides all this, I'd like to propose as well an extension point, for those who need a specific attribute but can't wait for a new release (PR check, Merge, Release) and therefore we'd keep this IDE and strongly typed lib friendly but it still opened.

// something similar to this
P1 = MPD.raw()[0]["Period"];
P2 = MPD.raw()[1]["Period"];

fmt.Printf(P1["start"]);

PS: I'm kinda new to golang so forgive me if my ideas are not so tied to the go's style.

[miyukki]

@stuarthicks @leandromoreira
I think another solution to resolve this issue.

1. Create MPDAttr interface and implement it as each attributes.

type MPDAttr interface {
	GetStrptr() *string
}

type AvailabilityStartTime struct {
	strptr *string
}
func (attr *AvailabilityStartTime) GetStrptr() *string {
	return attr.strptr
}
func AvailabilityStartTimeAttr(value string) MPDAttr {
	return &AvailabilityStartTime{strptr: &value}
}

type MediaPresentationDuration struct {
	strptr *string
}
func (attr *MediaPresentationDuration) GetStrptr() *string {
	return attr.strptr
}
func MediaPresentationDurationAttr(value string) MPDAttr {
	return &MediaPresentationDuration{strptr: &value}
}

2. NewMPD receive the MPDAttr as variable length argument and set the strptr from attributes. (minBufferTime is mandatory)

func NewMPD(profile DashProfile, minBufferTime string, attributes ...MPDAttr) *MPD {
	period := &Period{}
	mpd := &MPD{
		XMLNs:    Strptr("urn:mpeg:dash:schema:mpd:2011"),
		Profiles: Strptr((string)(profile)),
		Type:     Strptr("static"),
		MinBufferTime:             Strptr(minBufferTime),
		period:                    period,
		Periods:                   []*Period{period},
	}

	for _, attr := range attributes {
		switch attr.(type) {
		case *AvailabilityStartTime:
			mpd.AvailabilityStartTime = attr.GetStrptr()
		case *MediaPresentationDuration:
			mpd.MediaPresentationDuration = attr.GetStrptr()
		}
	}

	return mpd
}

func Example() {
	NewMPD(DASH_PROFILE_LIVE, "PT5S")
	
	NewMPD(DASH_PROFILE_LIVE, "PT5S",
		AvailabilityStartTimeAttr("PT0S"),
		MediaPresentationDurationAttr("PT0S"))
}

If you are afraid breaking API change, using NewDynamicMPD instead of.

[stuarthicks]

@miyukki That's a great suggestion, thanks! Very elegant. The only change I'd suggest is a stylistic one, and that would be to make the attributes have Attr as a prefix in the names, rather than a suffix, eg. AttrAvailabilityStartTime. It groups them nicely together when searching for attributes, in the same way errors are often named like ErrFoo.

@leandromoreira Are you happy to re-work this PR like that? What do you think about this new idea?

[miyukki]

@stuarthicks @leandromoreira
I opened another PR to resolve this, for using this feature in my project. #51

[stuarthicks]

Replaced by #51