Question about generating and parsing Multi Period Dash

[vish91]

Hey guys, I have a quick question about the <MPD> object structure and def. I think I understand that we have period and Periods to be backwards compatible I think for those who want to generate a single period dash without the ID ?
My question is around the multi period dash where an extra and empty Period is getting attached when a NewMPD() is being created.
e.g
I wrote this small test to show this

func TestNewMPDMultiPeriod(t *testing.T) {
	m := NewMPD(DASH_PROFILE_LIVE, VALID_MEDIA_PRESENTATION_DURATION, VALID_MIN_BUFFER_TIME,
		AttrAvailabilityStartTime(VALID_AVAILABILITY_START_TIME))
	require.NotNil(t, m)
	for i := 0; i < 2; i++ {
		period := m.AddNewPeriod()
		period.ID = strconv.Itoa(i)
	}
	ms, _ := m.WriteToString()

	expectedMPD := &MPD{
		XMLNs:                     Strptr("urn:mpeg:dash:schema:mpd:2011"),
		Profiles:                  Strptr((string)(DASH_PROFILE_LIVE)),
		Type:                      Strptr("static"),
		MediaPresentationDuration: Strptr(VALID_MEDIA_PRESENTATION_DURATION),
		MinBufferTime:             Strptr(VALID_MIN_BUFFER_TIME),
		AvailabilityStartTime:     Strptr(VALID_AVAILABILITY_START_TIME),
		period:                    nil,
		Periods:                   []*Period{&Period{ID: "0"}, &Period{ID: "1"}},
	}

	expectedString, err := expectedMPD.WriteToString()
	require.NoError(t, err)
	actualString, err := m.WriteToString()
	require.NoError(t, err)

	require.EqualString(t, expectedString, actualString)
}

which will give you an error

    require.go:99: Expected <?xml version="1.0" encoding="UTF-8"?>
        <MPD xmlns="urn:mpeg:dash:schema:mpd:2011" profiles="urn:mpeg:dash:profile:isoff-live:2011" type="static" mediaPresentationDuration="PT6M16S" minBufferTime="PT1.97S" availabilityStartTime="1970-01-01T00:00:00Z">
          <Period id="0"></Period>
          <Period id="1"></Period>
        </MPD>
         but got <?xml version="1.0" encoding="UTF-8"?>
        <MPD xmlns="urn:mpeg:dash:schema:mpd:2011" profiles="urn:mpeg:dash:profile:isoff-live:2011" type="static" mediaPresentationDuration="PT6M16S" minBufferTime="PT1.97S" availabilityStartTime="1970-01-01T00:00:00Z">
          <Period></Period>
          <Period id="0"></Period>
          <Period id="1"></Period>
        </MPD>

I think that extra <Period></Period> is unwanted and shouldn't be present in the manifest.

Let me know if this test makes sense ? Coz I see the other tests were written in a similar way but might make sense back in day before when it was a single period only support, so one would just assume that a an empty period is created with NewMPD() and we call GetCurrentPeriod() to fetch current period.

[thomshutt]

That does look like a valid bug to me, I'm not aware of any reason for the extra <Period> tag being there

[vish91]

:) Well luckily doesn't seem like a big change. All we need is to make sure when AddNewPeriod() is called we can check if there are any existing periods that are empty.
This will keep us backwards compatible as well so that people using single period do not have to make any changes to current behavior of NewMPD() which by default adds a new empty Period to MPD.
Opened a PR if you would like to take a look. ^
@stuarthicks it's October.. does this count as hacktoberfest :D