#2426 Support updating HLS Interstitials according to the HLS spec #2427

TomVarga · 2025-05-15T14:16:28Z

Use case description

According to the latest version of the HLS spec at https://datatracker.ietf.org/doc/html/draft-pantos-hls-rfc8216bis#section-4.4.5.1

If a Playlist contains two EXT-X-DATERANGE tags with the same ID
attribute value, then any AttributeName that appears in both tags
MUST have the same AttributeValue. A Server MAY augment a Date Range
with additional attributes by adding subsequent EXT-X-DATERANGE tags
with the same ID attribute to a Playlist. The client is responsible
for consolidating the tags. The subsequent EXT-X-DATERANGE tags can
appear in a subsequent playlist update, in the case of live or event
streams.

In a live stream scenario our system does updates to an interstitial according to the spec.

Here is simplified example stream:

https://raw.githubusercontent.com/TomVarga/HLS-test-stream/refs/heads/main/ride/audio.m3u8

Proposed solution

I am creating a PR to show my proposed solution, which is going to allow updating the interstitial, but still require the START-DATE for the first appearance of the interstitial.

This is a naive solution:

if an interstitial already appeared, then attempt to update it

Can use https://raw.githubusercontent.com/TomVarga/HLS-test-stream/refs/heads/main/ride/audio.m3u8 in the demo app to validate these changes

marcbaechinger · 2025-05-15T14:24:06Z

Thanks for the pull request.

I don't think we need changes in the parser or the HlsMediaPlaylist for this. This is exactly the same. In a live stream, the playlist is loaded again and then produces a playlist with the new interstitials.

The user of the playlist can then compare and update the interstitial by comparing playlistA with playlistB.

marcbaechinger · 2025-05-15T14:31:21Z

The change we would need for HlsINterstitialsAdsLoader would be that we let the ads loader updated the AdPlaybackState in [1] when we get the same interstitial again with changed attributes.

What does your stream update in the interstitial in the playlist?

[1] https://github.com/androidx/media/blob/main/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsInterstitialsAdsLoader.java#L1093

TomVarga · 2025-05-15T14:32:30Z

We need the code change because if you revert the changes in HlsPlaylistParser but run the test in HlsMediaPlaylistParserTest#parseMediaPlaylist_withInterstitialWithUpdatingDateRange you'll get:

Couldn't match START-DATE="(.+?)" in #EXT-X-DATERANGE:ID="15943",CLASS="com.apple.hls.interstitial",END-DATE="2024-09-20T15:29:49.006Z",X-PLAYOUT-LIMIT=24.953741497,X-RESUME-OFFSET=24.953741497 {contentIsMalformed=true, dataType=4}

Which is also what you get if you try and play this https://raw.githubusercontent.com/TomVarga/HLS-test-stream/refs/heads/main/ride/audio.m3u8 stream in the demo app.

Which means that the player is not handling the playlist according to the HLS spec as updating a daterange tag is allowed, meaning it's OK to not have START-DATE in a DATERANGE tag, as another DATERANGE tag can make changes to the interstitial based on the id

TomVarga · 2025-05-15T14:34:35Z

The change we would need for HlsINterstitialsAdsLoader would be that we let the ads loader updated the AdPlaybackState in [1] when we get the same interstitial again with changed attributes.

What does your stream update in the interstitial in the playlist?

[1] https://github.com/androidx/media/blob/main/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsInterstitialsAdsLoader.java#L1093

As you can see in the mentioned example stream, and the test I have added:

END-DATE
X-PLAYOUT-LIMIT
X-RESUME-OFFSET

get added to the interstitial

TomVarga · 2025-05-15T14:37:13Z

I have updated the PR description with the same as the issue to hopefully avoid some confusion.

marcbaechinger · 2025-05-15T14:46:39Z

We need the code change because if you revert the changes in HlsPlaylistParser

The model is that each time when a playlist is fetched an HlsMediaPlaylist object is created by the parser. If the interstitials have changed, then a new HlsMediaPlaylist is created with the updated content of the playlist.

We don't merge playlist A into playlist B or parts of it, so that multiple states of the playlist are represented.
A user can listen to onTimelineChanged() to get the playlist at time A. Then when it's fetched again at time B onTimelineChanged is called again with the next playlist. If the date ranges have changed the playlist will contain the new values. They won't be merged into the next playlist. For this you will have two instances of playlists with the state at time A and at time B.

If you see a failure with parsing please file a bug. This isn't related to updateability.

TomVarga · 2025-05-15T14:55:04Z

Sure, I can file the parsing issue as a bug, do please see #2428

marcbaechinger

Considering that I have misread the part 'If a Playlist contains two EXT-X-DATERANGE tags with' and haven't realized this in your unit test . I should have looked at this more closely beside the discussion around the START-DATE problems. Sorry for this.

So bseides the discussion about the spec ambiguity, this change makes totally sense in general as we want to support this according to spec at some point.

In the comment on L1321 I suggest to make an HlsMediaPlaylist.Interstitial.Builder because I think this would simplify the code and provide a good way to handle the update and validate merging the two or more tags.

Let me know what you think. It's fine to leave it to us to implement that Builder approach or convince me for another approach and why. :)

marcbaechinger · 2025-05-15T19:45:43Z

...es/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/HlsPlaylistParser.java

+        new ArrayList<>(interstitialMap.values()));
+  }
+
+  private static Interstitial getUpdatedInterstitial(


What about creating an HlsMediaPlaylist.Interstitial.Builder() that has setters for each field. When a setter is called multiple times, it asserts that it's the unset value or the same value else it throws.

Then we put the Interstitial.Builder instances in the LinkedHashMap by ID as you already do. At the very end, when we construct the HlsMediaPlaylist instance pass in the list of builder, and the constructor of Intersitital iterates over all builders, calls build() to get a ImmutableList of Interstitial instances.

I think that would make the code a bit simpler. WDYT?

marcbaechinger · 2025-05-15T19:47:01Z

...es/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/HlsPlaylistParser.java

+    // for consolidating the tags.  The subsequent EXT-X-DATERANGE tags can
+    // appear in a subsequent playlist update, in the case of live or event
+    // streams.
+    Uri newAssetUri = (assetUri != null) ? assetUri : oldInterstitial.assetUri;


I think the old value has precedence as mentioned in the Builder approach above or we should actually throw if not equal.

any AttributeName that appears in both tags MUST have the same AttributeValue.

...es/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/HlsPlaylistParser.java

marcbaechinger · 2025-05-15T19:58:18Z

...es/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/HlsPlaylistParser.java

-                  restrictions,
-                  clientDefinedAttributes.build()));
+
+        if (interstitialMap.containsKey(id)) {


If you do the Builder that I suggest below in the other comment, this if is at the very beginning on L1044 or so. Then you can set each value on the builder after parsing each field. The setter of the builder can then act accordingly: a) when the field wasn't set before: set the value b) if it was already set assert that the new value is either the same or unset, else throw. These validation can then be in the setter of each field. This can include things like checking that there is only an ASSET-URI or an ASSET-LIST.

marcbaechinger · 2025-05-15T20:17:51Z

A simple example of an Media3 Builder:

https://github.com/androidx/media/blob/release/libraries/common/src/main/java/androidx/media3/common/DeviceInfo.java#L58

TomVarga · 2025-05-16T17:36:31Z

Considering that I have misread the part 'If a Playlist contains two EXT-X-DATERANGE tags with' and haven't realized this in your unit test . I should have looked at this more closely beside the discussion around the START-DATE problems. Sorry for this.

So bseides the discussion about the spec ambiguity, this change makes totally sense in general as we want to support this according to spec at some point.

In the comment on L1321 I suggest to make an HlsMediaPlaylist.Interstitial.Builder because I think this would simplify the code and provide a good way to handle the update and validate merging the two or more tags.

Let me know what you think. It's fine to leave it to us to implement that Builder approach or convince me for another approach and why. :)

I'll update the PR with the builder approach.

TomVarga · 2025-05-19T14:52:30Z

I've pushed the requested changes, do please let me know if you think something is missing or needs improvement.

marcbaechinger

Nice! Thank you very much!

There are two or three things that we need to look at.

The rest are nits or uber-nits, as I gave you the hard way like I'd do internally. We can share these things also when leave some of them to me and I can do these nits when preparing for internal review. :)

...ies/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/HlsMediaPlaylist.java

marcbaechinger · 2025-05-19T16:52:26Z

...ies/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/HlsMediaPlaylist.java

+      public Builder setClientDefinedAttributes(
+          List<HlsMediaPlaylist.ClientDefinedAttribute> clientDefinedAttributes) {
+        if (clientDefinedAttributes.isEmpty()) return this;
+        if (!this.clientDefinedAttributes.isEmpty()) {


Not sure how to interpret the spec. These attributes are not interpreted by a player by spec, so changing them would not have any implications for playback. We could choose to be a bit less strict here.

From the text of the spec, we could as an alternative to your proposal allow any new attributes, but make sure we don't override existing ones with a different value. Easiest would probably to have a Map here in the builder keyed by the attribute.name and then check for equality if an attribute with the same name is provided an additional time. Then when build() is called this is converted to a list.

Being a bit less strict would mean to just make sure to not create duplicate names and either ignore the new attribute or allow it to be overridden. So being less strict gives us the burden to decide exactly that which we may want to avoid.

So I think I'd vote for checking equality. :)

WDYT?

I think we definitely need to allow new attributes to the list.

I agree probably easiest is to have a Map as you've described.

I also agree I'd rather check value equality too.

I'll add this in a follow up commit.

I have added some validation, do let me know if it looks OK to you.

...es/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/HlsPlaylistParser.java

...yer_hls/src/test/java/androidx/media3/exoplayer/hls/playlist/HlsMediaPlaylistParserTest.java

...ies/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/HlsMediaPlaylist.java

TomVarga · 2025-05-20T11:23:51Z

I think I addressed most of your comments, I'll work on a commit for clientDefinedAttributes. Let me know if it looks OK so far.

marcbaechinger

Very nice! Thank you very much!

I'm going to take this to send it for internal review.

Can you please let me quickly know that's fine? Looks to me you applied the changes for the client defined attributes.

TomVarga · 2025-05-20T20:34:56Z

Very nice! Thank you very much!

I'm going to take this to send it for internal review.

Can you please let me quickly know that's fine? Looks to me you applied the changes for the client defined attributes.

Yes, that's fine. Thank you.

… spec - if an interstitial already appeared, then attempt to update it

- use a builder - don't throw parse exception for missing START-DATE instead ignore the interstitial

… spec - some code style cleanup - fix negated expression in checkArguments and add test for it

… spec - add validation for clientDefinedAttribute

marcbaechinger · 2025-05-20T21:14:20Z

I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks!

marcbaechinger self-requested a review May 15, 2025 14:21

TomVarga mentioned this pull request May 15, 2025

HlsPlaylistParser fails to parse when EXT-X-DATERANGE does not have START-DATE #2428

Open

1 task

marcbaechinger reviewed May 15, 2025

View reviewed changes

microkatz assigned marcbaechinger May 16, 2025

marcbaechinger requested changes May 19, 2025

View reviewed changes

TomVarga requested a review from marcbaechinger May 20, 2025 12:09

marcbaechinger reviewed May 20, 2025

View reviewed changes

TomVarga and others added 5 commits May 20, 2025 23:09

androidx#2426 Support updating HLS Interstitials according to the HLS…

fab6c4c

… spec - if an interstitial already appeared, then attempt to update it

Support updating HLS Interstitials according to the HLS spec

dbdb885

- use a builder - don't throw parse exception for missing START-DATE instead ignore the interstitial

androidx#2426 Support updating HLS Interstitials according to the HLS…

8586d50

… spec - some code style cleanup - fix negated expression in checkArguments and add test for it

androidx#2426 Support updating HLS Interstitials according to the HLS…

521a91e

… spec - add validation for clientDefinedAttribute

Format with google-java-format

d580062

marcbaechinger force-pushed the tv/2426-Support-updating-HLS-Interstitials-according-to-the-HLS-spec branch from 3efb6a9 to d580062 Compare May 20, 2025 21:12

Add release notes

0f5ec84

marcbaechinger added the should merge label May 20, 2025

marcbaechinger added 2 commits May 21, 2025 00:53

minor cleanup

33f8b06

Minor cleanup

e2ed67f

marcbaechinger added 5 commits May 21, 2025 01:01

Ensure equality of interstitial

0f4cda4

Fix typo

170dd71

Resolve warnings from internal linter

6b167de

Minor reformatting for consistency

4fa449e

Address review comments

16b9880

marcbaechinger force-pushed the tv/2426-Support-updating-HLS-Interstitials-according-to-the-HLS-spec branch from 9050266 to 16b9880 Compare May 22, 2025 16:09

copybara-service bot merged commit b98e43f into androidx:main May 22, 2025
1 check passed

androidx locked and limited conversation to collaborators Jul 21, 2025

#2426 Support updating HLS Interstitials according to the HLS spec #2427

#2426 Support updating HLS Interstitials according to the HLS spec #2427

Uh oh!

Conversation

TomVarga commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Use case description

Proposed solution

Uh oh!

marcbaechinger commented May 15, 2025

Uh oh!

marcbaechinger commented May 15, 2025

Uh oh!

TomVarga commented May 15, 2025

Uh oh!

TomVarga commented May 15, 2025

Uh oh!

TomVarga commented May 15, 2025

Uh oh!

marcbaechinger commented May 15, 2025

Uh oh!

TomVarga commented May 15, 2025

Uh oh!

marcbaechinger left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcbaechinger May 15, 2025

Choose a reason for hiding this comment

Uh oh!

marcbaechinger May 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marcbaechinger May 15, 2025

Choose a reason for hiding this comment

Uh oh!

marcbaechinger commented May 15, 2025

Uh oh!

TomVarga commented May 16, 2025

Uh oh!

TomVarga commented May 19, 2025

Uh oh!

marcbaechinger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marcbaechinger May 19, 2025

Choose a reason for hiding this comment

Uh oh!

TomVarga May 20, 2025

Choose a reason for hiding this comment

Uh oh!

TomVarga May 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TomVarga commented May 20, 2025

Uh oh!

marcbaechinger left a comment

Choose a reason for hiding this comment

Uh oh!

TomVarga commented May 20, 2025

Uh oh!

marcbaechinger commented May 20, 2025

Uh oh!

Uh oh!

Uh oh!

TomVarga commented May 15, 2025 •

edited

Loading

marcbaechinger left a comment •

edited

Loading