Closed Bug 1211328 Opened 4 years ago Closed 4 years ago

When sourcebuffer.mode = sequence, timestampOffset is incorrectly calculated

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox42 --- fixed
firefox43 --- fixed
firefox44 --- fixed

People

(Reporter: jya, Assigned: jya)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Priority: -- → P1
Version: 43 Branch → Trunk
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.
Attachment #8669513 - Flags: review?(gsquelart) → review+
https://hg.mozilla.org/mozilla-central/rev/40ddc543dd16
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
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
Attachment #8669513 - Flags: approval-mozilla-beta?
Attachment #8669513 - Flags: approval-mozilla-aurora?
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)
Attachment #8669513 - Flags: approval-mozilla-beta?
Attachment #8669513 - Flags: approval-mozilla-beta+
Attachment #8669513 - Flags: approval-mozilla-aurora?
Attachment #8669513 - Flags: approval-mozilla-aurora+
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
Blocks: 1211652
You need to log in before you can comment on or make changes to this bug.