Skip to content

[LL-HLS] Prevent excessive reloads when CAN-BLOCK-RELOAD=YES is not respected #2317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[LL-HLS] Prevent excessive reloads when CAN-BLOCK-RELOAD=YES is not respected #2317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions RELEASENOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@
* Cronet extension:
* RTMP extension:
* HLS extension:
* Prevent excessive reloads by waiting for half the part target duration
when `CAN-BLOCK-RELOAD=YES` is not honored.
* DASH extension:
* Smooth Streaming extension:
* RTSP extension:
Expand Down
7 changes: 7 additions & 0 deletions ...r_hls/src/main/java/androidx/media3/exoplayer/hls/playlist/DefaultHlsPlaylistTracker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,13 @@ private void processLoadedPlaylist(
playlistSnapshot != oldPlaylist
? playlistSnapshot.targetDurationUs
: (playlistSnapshot.targetDurationUs / 2);
} else if (
playlistSnapshot == oldPlaylist && playlistSnapshot.partTargetDurationUs != C.TIME_UNSET
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason you only do this if a part target duration is defined? The same should apply to playlists that canBlockReload but are not low-latency I assume.

Related to that: the fallback should probably be to use playlistSnapshot.targetDurationUs / 2 as in the if case above for when canBlockReload is not supported.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @tonihei,

First of all, thank you for reviewing the PR positively.

The IETF standard RFC 8216 does not mention CAN-BLOCK-RELOAD at all. This attribute is introduced in the context of LL-HLS and described in Section 6.2.5.2 of draft-pantos-hls-rfc8216bis. According to the specification, the use of _HLS_msn and _HLS_part parameters—required for blocking playlist reload behavior—appears to clearly limit this mechanism to LL-HLS.

6.2.5.2.  Blocking Playlist Reload

   A Server MAY offer Blocking Playlist Reloads, which enable immediate
   client discovery of Playlist updates as an alternative to polling.

   A Server advertises support for Blocking Playlist Reload by adding
   the CAN-BLOCK-RELOAD=YES attribute to the EXT-X-SERVER-CONTROL tag.

   A Client requests a Blocking Playlist Reload using an _HLS_msn
   directive with a decimal-integer value M.  When the Playlist URI
   contains an _HLS_msn directive and no _HLS_part directive, the Server
   MUST defer responding to the request until the Playlist contains a
   Media Segment with a Media Sequence Number of M or later or it
   responds with an error.

That said, I understand and agree with your concern, as this PR handles an edge case where the server does not fully comply with the RFC.

As a refinement, I’d suggest the following logic:

} else if (playlistSnapshot == oldPlaylist) {
  // To prevent infinite requests when the server responds with CAN-BLOCK-RELOAD=YES but does
  // not actually block until the playlist updates, wait before retrying.
  durationUntilNextLoadUs = playlistSnapshot.partTargetDurationUs != C.TIME_UNSET
      ? playlistSnapshot.partTargetDurationUs / 2
      : playlistSnapshot.targetDurationUs / 2;
}

Copy link
Author

Choose a reason for hiding this comment

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

If you have any thoughts or suggestions on this approach, I’d be happy to revise the code accordingly.

Also, would you prefer that I squash the changes into a single commit and force-push, or keep them as separate commits?

) {
// To prevent infinite requests when the server responds with CAN-BLOCK-RELOAD=YES but does
// not actually block until the playlist updates, wait for half the part target duration
// before retrying.
durationUntilNextLoadUs = playlistSnapshot.partTargetDurationUs / 2;
}
earliestNextLoadTimeMs =
currentTimeMs + Util.usToMs(durationUntilNextLoadUs) - loadEventInfo.loadDurationMs;
Expand Down