Retry aligning audio playlist with main playlist if levelloaded came too early

[Frenzie]

This will retry aligning the audio playlist with the main playlist if levelloaded came too early

Why is this Pull Request needed?

The problem is that sometimes the onLevelLoaded event comes just after onAudioTrackLoaded, and then everything has to wait around for the entire segment time before it finally gets going.

A very simple workaround looks like this, but that has some rather obvious downsides:

diff --git a/src/controller/audio-stream-controller.ts b/src/controller/audio-stream-controller.ts
index e9fa2d1c..91e16dc9 100644
--- a/src/controller/audio-stream-controller.ts
+++ b/src/controller/audio-stream-controller.ts
@@ -433,7 +436,14 @@ class AudioStreamController
	   if (!newDetails.fragments[0]) {
		 newDetails.deltaUpdateFailed = true;
	   }
	   if (newDetails.deltaUpdateFailed || !mainDetails) {
+        setTimeout(() => {
+          this.hls.trigger(Events.AUDIO_TRACK_LOADED, data)
+        }, 100)
		 return;
	   }
	   if 

Instead I modeled it on the levelLoadingRetryDelay code as in base-playlist-controller which deals with a similar scenario.

hls.js/src/controller/base-playlist-controller.ts

Lines 239 to 264 in 5039b23

	const { config } = this.hls;
	const retry = this.retryCount < config.levelLoadingMaxRetry;
	if (retry) {
	this.retryCount++;
	if (
	errorEvent.details.indexOf('LoadTimeOut') > -1 &&
	errorEvent.context?.deliveryDirectives
	) {
	// The LL-HLS request already timed out so retry immediately
	this.warn(
	`retry playlist loading #${this.retryCount} after "${errorEvent.details}"`
	);
	this.loadPlaylist();
	} else {
	// exponential backoff capped to max retry timeout
	const delay = Math.min(
	Math.pow(2, this.retryCount) * config.levelLoadingRetryDelay,
	config.levelLoadingMaxRetryTimeout
	);
	// Schedule level/track reload
	this.timer = self.setTimeout(() => this.loadPlaylist(), delay);
	this.warn(
	`retry playlist loading #${this.retryCount} in ${delay} ms after "${errorEvent.details}"`
	);
	}
	} else {

Are there any points in the code the reviewer needs to double check?

There may be a much better way to do this. In my testing the mistiming only seems to be very slight, so even the default levelLoadingRetryDelay of 1000 ms is rather long. 100 ms does the trick without being noticeable.

# audiotrack loaded, too early
12:02:00.316 [log] > [audio-stream-controller]: Track 0 loaded [1673,1677],duration:30 [hls.js:5914:10](http://213.219.154.49:8000/dist/hls.js)

# new addition to retry in a bit rather than in 10 seconds or whatever the segment length is
12:02:00.317 [warn] > [audio-stream-controller]: retry audiotrack loading #0 in 1000 ms [hls.js:5938:14](http://213.219.154.49:8000/dist/hls.js)

# level loaded, only 10 ms later
12:02:00.327 [log] > [level-controller]: reload live playlist 2 in 5957 ms [hls.js:6781:12](http://213.219.154.49:8000/dist/hls.js)
12:02:00.327 [log] > [stream-controller]: Level 2 loaded [1673,1677], cc [0, 0] duration:30 [hls.js:13827:10](http://213.219.154.49:8000/dist/hls.js)

Additionally, whatever the best solution may be, it should presumably be copied more or less verbatim to subtitle-stream-controller. I haven't done testing with that yet, plus as I said I'm not sure if this PR as currently written is necessarily the way to go.

hls.js/src/controller/subtitle-stream-controller.ts

Lines 249 to 252 in 5039b23

	const mainDetails = this.mainDetails;
	if (newDetails.deltaUpdateFailed || !mainDetails) {
	return;
	}

Resolves issues:

Fixes #4288.

Checklist

• changes have been done against master branch, and PR does not conflict
• new unit / functional tests have been added (whenever applicable)
• API or design changes are documented in API.md

[Frenzie]

(Rebased against current master, mainly to avoid the X; cf. #4604.)

[Frenzie]

Activity.

[robwalch]

Not a Contribution

Hi @Frenzie,

A simpler solution has been suggested here: #4631 (comment)

Would you be willing to make the changes to adopt this approach?

[Frenzie]

@robwalch Sure, but unfortunately I won't have time within the next two weeks.

[kmikodev]

Hello!! I have a question, what if the bitrate request is still over 100ms? @Frenzie

[Frenzie]

@kmikodev If you're talking about this PR in its current form, the 100 ms I mentioned was merely my initial proof of concept that isn't part of the actual diff.

I then thought about what would be the best way to do it properly, both in terms of the desired behavior and what would fit in with existing code, and landed on copying base-playlist-controller pretty much verbatim since it does exactly what I wanted.

        const delay = Math.min(
          Math.pow(2, this.retryCount) * config.levelLoadingRetryDelay,
          config.levelLoadingMaxRetryTimeout
        );

config.levelLoadingRetryDelay defaults to 1000 and then eases off depending on the number of retries. I was merely saying that in my testing there seemed to be no reason to default to a value higher than 100 for this specific scenario, but the difference between starting after 100 ms and starting after 1000 ms isn't very noticeable unless you're testing them side by side. The important thing is that you're not waiting for the entire segment size of for example 10 s.

But while I don't have time to investigate right now, it looks like it should suffice to simply trigger the AUDIO_TRACK_LOADED event on level loaded. In theory that's a much simpler and more elegant solution. I'll just need to do extensive testing to verify it indeed works just as well.

[Frenzie]

@robwalch Updated as requested. In my testing all seems well.

[robwalch]

Not a Contribution

Hi @Frenzie,

Can you look at the failed unit test? It looks like mainDetails needs to be mocked on the audio track controller.

[Frenzie]

@robwalch This one-line change is all that's needed to pass the test, but I'm not sure if there are any potential pitfalls.