1211328 - When sourcebuffer.mode = sequence, timestampOffset is incorrectly calculated

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Jean-Yves Avenard [:jya]]

Per spec :http://w3c.github.io/media-source/index.html#sourcebuffer-coded-frame-processing

when mode = sequence: Set timestampOffset equal to group start timestamp - presentation timestamp.

This is required to ensure that when in sequence mode: "The timestampOffset attribute will be updated if a new offset is needed to make the new media segments adjacent to the previous media segment. " (http://w3c.github.io/media-source/index.html#idl-def-AppendMode.sequence).

However, our implementation set timestampOffset to group start timestamp ; and as such is always positive.

This result in the next segment being appended to be incorrectly added and create an even greater discontinuity.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: [MSE] Fix timestampOffset attribute calculation in sequence mode.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8669513 [details] [diff] [review]
[MSE] Fix timestampOffset attribute calculation in sequence mode.

Review of attachment 8669513 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with nit.

::: dom/media/mediasource/TrackBuffersManager.cpp
@@ +1353,5 @@
>    mProcessingPromise.ResolveIfExists(aResolveValue, __func__);
>  }
>  
>  void
> +TrackBuffersManager::CheckSequenceDiscontinuity(const media::TimeUnit& aPresentationTime)

No need for "media::" here.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/40ddc543dd16

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/40ddc543dd16

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8669513 [details] [diff] [review]
[MSE] Fix timestampOffset attribute calculation in sequence mode.

Approval Request Comment
[Feature/regressing bug #]: 1171330. 42 is the first release officially supporting this API.
[User impact if declined]: Invalid behaviour of DOM MSE function, media playback stalls.
[Describe test coverage new/current, TreeHerder]: local test, webref, extra test pages created.
[Risks and why]: Nonexistent, only updating a variable as per spec.
[String/UUID change made/needed]: None

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8669513 [details] [diff] [review]
[MSE] Fix timestampOffset attribute calculation in sequence mode.

Polish MSE, taking in beta (should be in 42 beta 4)

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/20714e18573b

Comment 8

[Jean-Yves Avenard [:jya]]

Here is a test page to reproduce the issue:
http://people.mozilla.org/~jyavenard/tests/mse_mp4/sequence-test.html

If the problem is fixed, the video.buffered showing should be continuous, and sourceBuffer.timestampOffset should have a value < 0.

e.g.:
Bad:
398: video.buffered ={{ 0,0.801666 }{ 4.004999,4.806666 }}
398: sourceBuffer.timestampOffset = 0.801666

Good:
886: video.buffered ={{ 0,1.603333 }}
887: sourceBuffer.timestampOffset = -2.401667

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b6718afe48be