Support for reading/writing EXT-X-BYTERANGE tag, Enable unknown tags when writing playlists #47
Conversation
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
changed the title
| 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 + "))?$"); |
There was a problem hiding this comment.
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()) { | |||
| lineParser.parse(line, state); | ||
| final Matcher matcher = ParseUtil.match(Constants.EXT_X_BYTE_RANGE_PATTERN, line, getTag()); | ||
|
|
||
| if (matcher.groupCount() == 3) { |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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)); |
There was a problem hiding this comment.
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.
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.