Skip to content

Commit 8cd2de7

Browse files
committed
Fix yet another issue with those Tizen seek-backs
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.
1 parent 169e6d6 commit 8cd2de7

File tree

1 file changed

+19
-9
lines changed

1 file changed

+19
-9
lines changed

src/main_thread/init/utils/rebuffering_controller.ts

+19-9
Original file line numberDiff line numberDiff line change
@@ -171,15 +171,16 @@ export default class RebufferingController extends EventEmitter<IRebufferingCont
171171
if (position.isAwaitingFuturePosition()) {
172172
playbackRateUpdater.stopRebuffering();
173173
log.debug("Init: let rebuffering happen as we're awaiting a future position");
174-
this.trigger("stalled", stalledReason);
175-
return;
174+
} else {
175+
playbackRateUpdater.startRebuffering();
176176
}
177177

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

193+
/**
194+
* We may still be in the process of waiting for a position to be seeked
195+
* to. When calculating a potential position to e.g. skip over
196+
* discontinuities, we should compare it to that "target" position if
197+
* one, not the one we're currently playing.
198+
*/
199+
const targetTime = observation.position.isAwaitingFuturePosition()
200+
? observation.position.getWanted()
201+
: this._playbackObserver.getCurrentTime();
202+
192203
if (
193204
stalledPosition !== null &&
194205
stalledPosition !== undefined &&
@@ -201,10 +212,10 @@ export default class RebufferingController extends EventEmitter<IRebufferingCont
201212
);
202213
if (skippableDiscontinuity !== null) {
203214
const realSeekTime = skippableDiscontinuity + 0.001;
204-
if (realSeekTime <= this._playbackObserver.getCurrentTime()) {
215+
if (realSeekTime <= targetTime) {
205216
log.info(
206217
"Init: position to seek already reached, no seeking",
207-
this._playbackObserver.getCurrentTime(),
218+
targetTime,
208219
realSeekTime,
209220
);
210221
} else {
@@ -243,7 +254,7 @@ export default class RebufferingController extends EventEmitter<IRebufferingCont
243254
nextBufferRangeGap < BUFFER_DISCONTINUITY_THRESHOLD
244255
) {
245256
const seekTo = positionBlockedAt + nextBufferRangeGap + EPSILON;
246-
if (this._playbackObserver.getCurrentTime() < seekTo) {
257+
if (targetTime < seekTo) {
247258
log.warn(
248259
"Init: discontinuity encountered inferior to the threshold",
249260
positionBlockedAt,
@@ -266,8 +277,7 @@ export default class RebufferingController extends EventEmitter<IRebufferingCont
266277
if (period.end !== undefined && period.end <= positionBlockedAt) {
267278
if (
268279
this._manifest.periods[i + 1].start > positionBlockedAt &&
269-
this._manifest.periods[i + 1].start >
270-
this._playbackObserver.getCurrentTime()
280+
this._manifest.periods[i + 1].start > targetTime
271281
) {
272282
const nextPeriod = this._manifest.periods[i + 1];
273283
this._playbackObserver.setCurrentTime(nextPeriod.start);

0 commit comments

Comments
 (0)