Skip to content
This repository has been archived by the owner on Nov 22, 2022. It is now read-only.

Support for reading/writing EXT-X-BYTERANGE tag, Enable unknown tags when writing playlists #47

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

krol01
Copy link

@krol01 krol01 commented Jun 12, 2017

When a media playlist builder is constructed to use unknown tags (MediaPlaylist.Builder().withUnknownTags(...)) the unknown tags will not be written using the ExtendedM3uWriter.
This commit enables the EXT_UNKNOWN_HANDLER and fixes a potential NullPointerException.

When a media playlist builder is configured to use unknown tags (MediaPlaylist.Builder().withUnknownTags(...))
the unknown tags will not be written using the ExtendedM3uWriter, this commit enables the EXT_UNKNOWN_HANDLER
and fixes a potential NullPointerException.
@krol01 krol01 changed the title Enable unknown tags when writing playlists Support for reading/writing EXT-X-BYTERANGE tag, Enable unknown tags when writing playlists Jun 14, 2017
public static final Pattern EXT_X_MEDIA_IN_STREAM_ID_PATTERN = Pattern.compile("^CC[1-4]|SERVICE(?:[1-9]|[1-5]\\d|6[0-3])$");
public static final Pattern EXTINF_PATTERN = Pattern.compile("^#" + EXTINF_TAG + EXT_TAG_END + "(" + SIGNED_FLOAT_REGEX + ")(?:,(.+)?)?$");
public static final Pattern EXT_X_BYTE_RANGE_PATTERN = Pattern.compile("^#" + EXT_X_BYTE_RANGE_TAG + EXT_TAG_END + "(" + INTEGER_REGEX + ")(@(" + INTEGER_REGEX + "))?$");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of:

"(@(" + INTEGER_REGEX + "))?$"

you can do:

"(?:@(" + INTEGER_REGEX + "))?$"

to not capture the second group with the '@'. Then the parsing becomes a little more straightforward.

@@ -63,7 +63,7 @@ public void write(TagWriter tagWriter, Playlist playlist) throws IOException {
List<String> unknownTags;
if (playlist.hasMasterPlaylist() && playlist.getMasterPlaylist().hasUnknownTags()) {
unknownTags = playlist.getMasterPlaylist().getUnknownTags();
} else if (playlist.getMediaPlaylist().hasUnknownTags()) {
} else if (playlist.hasMediaPlaylist() && playlist.getMediaPlaylist().hasUnknownTags()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

lineParser.parse(line, state);
final Matcher matcher = ParseUtil.match(Constants.EXT_X_BYTE_RANGE_PATTERN, line, getTag());

if (matcher.groupCount() == 3) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not believe there is a case where the group count can be anything other than the total number of groups. Even if that group is empty, group count should return the same number. I think we can remove the if condition and keep the code in the block since it's guaranteed to have a match after the call to match above.


public class ByteRange {
private final Integer subRangeLength;
private final Integer offset;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data pojos have been using primitives wherever possible. This is partly to reduce the risk of nulls being created. I see that null is indeed possible in the parsing above. I will comment on how to better handle that.


if (matcher.groupCount() == 3) {
String offset = matcher.group(3);
state.getMedia().byteRange = new ByteRange(Integer.parseInt(matcher.group(1)), offset == null ? null : Integer.parseInt(offset));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec says the following:

If o is not present, the sub-range begins at the next byte following
the sub-range of the previous Media Segment.

If o is not present, a previous Media Segment MUST appear in the
Playlist file and MUST be a sub-range of the same media resource, or
the Media Segment is undefined and the Playlist MUST be rejected.

This is a bit unfortunate since it means we will need to add a piece of state to the parsing, but it does mean there is no null case. We need to keep track of the last byte range (which may be nothing) to calculate the offset for the current byte range if no offset is specified. So it would look something like this:

offset = group(2);

if (offset is null) {
    if (previous is null) {
        throw;
    } else {
        offset = previous.offset + previous.subRangeLength;
    }
}

We're not quite done. Because of this:

... a previous Media Segment MUST appear in the
Playlist file and MUST be a sub-range of the same media resource ...

we need to clear the previous byte range when changing media sources.

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

Successfully merging this pull request may close these issues.

2 participants