Skip to content

Commit

Permalink
Fix yet another issue with those Tizen seek-backs
Browse files Browse the repository at this point in the history
This is approximately the 9001th commit trying to work-around bugs
arising due to the fact that Tizen doesn't let us seek where we want to
seek in a misguided attempt to improve performance (which actually just
do the opposite here).

Here #1635 told us about yet another issue where playback doesn't begin
when there's a discontinuity right at the beginning. After looking at
communicated logs, I think this was because of a very unfortunate chain
of events:

  1. We begin to play a content with both audio and video segments
     (creating both an audio and video SourceBuffer) accordingly

  2. Audio segments are the first one to be loaded. We push them to the
     audio SourceBuffer

  3. The audio SourceBuffer later report to us that what it has buffered
     begins at `0.016`, not `0`.

     We have no problem with that. However Tizen seems to be more broken
     than we thought from our understanding of the HTML spec because
     then `HTMLMediaElement.prototype.buffered` also report data
     beginning at `0.016` despite the fact that video hasn't been
     currently be pushed in its own SourceBuffer (so to me the "global"
     `buffered` should be empty).

  4. Our `RebufferingController` sees that there's a discontinuity at
     `0.016` and thus seeks just over it.

  5. We're now pushing the first video segments. Because we're unlucky,
     they begin even later, at `0.08`.

  6. Now the "global" `buffered` speaks some sense: it is now the
     intersection of the audio and video SourceBuffers like we expect
     and thus it now begins at `0.08`

  7. Our `RebufferingController` do not skip that now updated
     discontinuity because it is still in the process of skipping the
     (actually-not-real) `0.016` one and because it knows we're on a
     device on which seeking is broken, so it just waits until Tizen
     has processed that first seek..

  8. Our `PlaybackObserver`, which gives its opinion on whether we
     should be paused to buffer content, sees that the target position
     is still `0.016` so start its logic from there. At `0.016`, there's
     no data (it begins at `0.08` now), so for it, we should still wait
     for segments to be loaded.

  9. Our "initial_seek_and_play" logic, which between other things
     perform the initial `play` call, thus see that for the
     `PlaybackObserver`, we should wait. It thus waits until the
     `PlaybackObserver` give the greenlight to start the content.

To fix this, I updated the behavior of the `RebufferingController` on
Tizen so that it continues checking for discontinuities even if we're
already waiting for one to be skipped.

I though had to not consider the currently-played position anymore but
actually the perhaps-different "target" position we should be playing if
the device wasn't as broken as Tizen is.

To note that this bug perhaps showed us another Tizen issue which is the
`buffered` thing (though perhaps I'm not up-to-date with the specs?). I
didn't fix it at this level because it's much harder.
  • Loading branch information
peaBerberian committed Jan 24, 2025
1 parent 169e6d6 commit 8cd2de7
Showing 1 changed file with 19 additions and 9 deletions.
28 changes: 19 additions & 9 deletions src/main_thread/init/utils/rebuffering_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,15 +171,16 @@ export default class RebufferingController extends EventEmitter<IRebufferingCont
if (position.isAwaitingFuturePosition()) {
playbackRateUpdater.stopRebuffering();
log.debug("Init: let rebuffering happen as we're awaiting a future position");
this.trigger("stalled", stalledReason);
return;
} else {
playbackRateUpdater.startRebuffering();
}

playbackRateUpdater.startRebuffering();

if (
this._manifest === null ||
(isSeekingApproximate &&
// Don't handle discontinuities on devices with broken seeks before
// enough time have passed because seeking brings more risks to
// lead to a lengthy rebuffering-exiting process
getMonotonicTimeStamp() - rebuffering.timestamp <= 1000)
) {
this.trigger("stalled", stalledReason);
Expand All @@ -189,6 +190,16 @@ export default class RebufferingController extends EventEmitter<IRebufferingCont
/** Position at which data is awaited. */
const { position: stalledPosition } = rebuffering;

/**
* We may still be in the process of waiting for a position to be seeked
* to. When calculating a potential position to e.g. skip over
* discontinuities, we should compare it to that "target" position if
* one, not the one we're currently playing.
*/
const targetTime = observation.position.isAwaitingFuturePosition()
? observation.position.getWanted()
: this._playbackObserver.getCurrentTime();

if (
stalledPosition !== null &&
stalledPosition !== undefined &&
Expand All @@ -201,10 +212,10 @@ export default class RebufferingController extends EventEmitter<IRebufferingCont
);
if (skippableDiscontinuity !== null) {
const realSeekTime = skippableDiscontinuity + 0.001;
if (realSeekTime <= this._playbackObserver.getCurrentTime()) {
if (realSeekTime <= targetTime) {
log.info(
"Init: position to seek already reached, no seeking",
this._playbackObserver.getCurrentTime(),
targetTime,
realSeekTime,
);
} else {
Expand Down Expand Up @@ -243,7 +254,7 @@ export default class RebufferingController extends EventEmitter<IRebufferingCont
nextBufferRangeGap < BUFFER_DISCONTINUITY_THRESHOLD
) {
const seekTo = positionBlockedAt + nextBufferRangeGap + EPSILON;
if (this._playbackObserver.getCurrentTime() < seekTo) {
if (targetTime < seekTo) {
log.warn(
"Init: discontinuity encountered inferior to the threshold",
positionBlockedAt,
Expand All @@ -266,8 +277,7 @@ export default class RebufferingController extends EventEmitter<IRebufferingCont
if (period.end !== undefined && period.end <= positionBlockedAt) {
if (
this._manifest.periods[i + 1].start > positionBlockedAt &&
this._manifest.periods[i + 1].start >
this._playbackObserver.getCurrentTime()
this._manifest.periods[i + 1].start > targetTime
) {
const nextPeriod = this._manifest.periods[i + 1];
this._playbackObserver.setCurrentTime(nextPeriod.start);
Expand Down

0 comments on commit 8cd2de7

Please sign in to comment.