Add SCTE-35 support #106
Add SCTE-35 support #106
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) ✅ code/snyk check is complete. No issues have been found. (View Details) |
mpd/scte35_test.go
Outdated
| actualString, err := m.WriteToString() | ||
| require.NoError(t, err) | ||
|
|
||
| require.EqualString(t, expectedString, actualString) |
There was a problem hiding this comment.
Rather than test this here, you should have a hardcoded mpd in in the fixtures/ folder, and compare the actualString to the fixture itself much like the event stream tests https://github.com/zencoder/go-dash/blob/master/mpd/events_test.go#L55
The way you have this assertion implemented can actually hide bugs. If there was a bug in say the xml tags you added such that the xml encoded value of the struct is invalid, you would never catch it here because both the expected and actual are generated by encoding a struct to xml string, both would encode with the same bug. Comparing against a static fixture can catch any encoding bugs.
There was a problem hiding this comment.
Updated the test to load the fixtures/scte35.mpd file as the expected mpd, and compared it to the generated one here: 3754381
mpd/fixtures/scte35.mpd
Outdated
| <EventStream schemeIdUri="urn:scte:scte35:2014:xml+bin" timescale="10"> | ||
| <Event id="1" presentationTime="10000"> | ||
| <Signal xmlns="urn:scte:scte35:2014:xml+bin"> | ||
| <Binary xmlns="urn:scte:scte35:2014:xml+bin"></Binary> |
There was a problem hiding this comment.
is it expected to still have a <Binary></Binary> tag if there is no data within it, or should we expect this to be omitted? not sure if it is required by the spec or to satisfy precondition requirements?
There was a problem hiding this comment.
Good point, updated the library, to only keep the Event field, as that contains the presentationTime for an event whenever the binary is empty (also updated a bunch of other stuff to better support scte-35).
Description
This pr adds support for SCTE-35 on the go-dash library.