EXT-X-DATERANGE support

theRealRobG commented 4 years ago

Description

This PR attempts to add EXT-X-DATERANGE as a tag that is recognised by Mamba under PantosTag, along with the attributes that the tag supports. The attributes were taken from draft-pantos-hls-rfc8216bis-05.

Change Notes

Added new case to PantosTag
Added new cases to PantosValue
Added validation for PantosTag.EXT_X_DATERANGE (EXT_X_DATERANGETagValidator)
Added playlist validation (EXT_X_DATERANGEPlaylistValidator)

There are some concerns I have with the changes:

PlaylistTagDescriptorScope

The PlaylistTagDescriptorScope has been defined to be .wholePlaylist, rather than .mediaSegment, or .mediaSpanning, but in actuality, it seems to be its own category entirely in the draft HLS specification, under "Media Metadata Tags". The following is said about this category in the specification:

 Media Metadata tags provide information about the playlist that is
 not associated with specific Media Segments.

As such, it seemed that the closest descriptive case of scope seems to be wholePlaylist; that being said, I don't fully understand the implications of the scope, and the tag actually is applied to a date range of the whole playlist. Perhaps a new case should be added... I will leave that up to a decision in the review.

Error severity

I'm not entirely sure what the error severity is actually supposed to indicate. I've chosen all of the validation errors for EXT-X-DATERANGE to be .error, since all the rules I have implemented are indicated as MUST in the specification. That being said, there is this line in the specification that indicates that any syntax error for a EXT-X-DATERANGE tag, should just be treated as the tag being ignored (so it is not an error for the whole playlist):

Clients SHOULD ignore EXT-X-DATERANGE tags with illegal syntax.

Some advice on what direction I should take with this would be appreciated.

Pre-submission Checklist

[x] I ran the unit tests locally before checking in.
[x] I made sure there were no compiler warnings before checking in.
[x] I have written useful documentation for all public code.
[x] I have written unit tests for this new feature.

CLAassistant commented 4 years ago

All committers have signed the CLA.

dcoufal commented 4 years ago

The main use of PlaylistTagDescriptorScope is for automatic creation of "structure" for parsed playlists. You may have noticed that we have header, mediaSegmentGroups, and footer groups of tags in HLSPlaylistInterface, as well as mediaSpans.

Given the nature of the tag, I'd say that .wholePlaylist is accurate. It does not describe a fragment nor a group of fragments.

dcoufal commented 4 years ago

Re: Error handling. Generally, a validation error should mean that the playlist is unplayable.

If it's explicitly stated that malformed EXT-X-DATERANGE are just ignored, I would drop them down to warnings.

Comcast / mamba