Closed Bug 1481397 Opened 2 years ago Closed 2 years ago

HighestStartTimestamp not reset under some circumstances...

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox61 --- wontfix
firefox62 --- fixed
firefox63 --- fixed

People

(Reporter: jya, Assigned: jya)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

The test http://w3c-test.org/media-source/mediasource-changetype-play.html fails.

Upon investigation the issue is due to failing this step in the change duration algorithm

https://w3c.github.io/media-source/index.html#duration-change-algorithm
"If new duration is less than the highest presentation timestamp of any buffered coded frames for all SourceBuffer objects in sourceBuffers, then throw an InvalidStateError exception and abort these steps.
"

the issue here is that following the call to the Range Removal Algorithm, the highest presentation timestamp value isn't updated as it should.

Causing later failure.
When removing frames from the trackbuffer we may remove frames outside the original removal interval as we must remove all frames depending on the removed frames.
Also, enable changeType MSE tests.

Depends on D2837
Comment on attachment 8998270 [details]
Bug 1481397 - P2. Update test expectations. r=jgraham

James Graham [:jgraham] has approved the revision.

https://phabricator.services.mozilla.com/D2870
Attachment #8998270 - Flags: review+
Comment on attachment 8998137 [details]
Bug 1481397 - Properly recalculate highest start timestamp when samples are removed. r?bryce

Bryce Seager van Dyk (:bryce) has approved the revision.

https://phabricator.services.mozilla.com/D2837
Attachment #8998137 - Flags: review+
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/edf9688dc9ac
P1. Properly recalculate highest start timestamp when samples are removed. r=bryce
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cf920a66ad4
P2. Update test expectations. r=jgraham
https://hg.mozilla.org/mozilla-central/rev/edf9688dc9ac
https://hg.mozilla.org/mozilla-central/rev/4cf920a66ad4
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
jya or nils, is this something you might want to uplift to beta 62?
Flags: needinfo?(drno)
This only appears to affect the change-type API, which I think is not turned on by default in 62. So I would say there isn't a need to uplift. But I'll let the expert jya speak to that.
Flags: needinfo?(drno) → needinfo?(jyavenard)
The bug was found in the changeType wpt, however the problem is unrelated to changeType and will affect all Firefox version: changing the duration of the element is not possible when partial data has been added under some circumstances. I don’t know how common that issue would be, but it would cause either a playback error or the inability for a video to ever reach its end, so the ended event won’t ever fire .

Uplifting this would be nice, but it’s likely too late.
Flags: needinfo?(jyavenard)
Comment on attachment 8998137 [details]
Bug 1481397 - Properly recalculate highest start timestamp when samples are removed. r?bryce

Approval Request Comment
[Feature/Bug causing the regression]: MSE
[User impact if declined]: error when setting the duration of a video element, or playback will stall as it can’t reach the end
[Is this code covered by automated tests?]: not exactly for this bug, but some wpt were failing because of this.
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: can craft a JS to present the problem.
[List of other uplifts needed for the feature/fix]: just p1
[Is the change risky?]: no
[Why is the change risky/not risky?]: it’s not working now and the change are very localised to the duration change algorithm. So it can only get better
[String changes made/needed]: none
Attachment #8998137 - Flags: approval-mozilla-beta?
Comment on attachment 8998137 [details]
Bug 1481397 - Properly recalculate highest start timestamp when samples are removed. r?bryce

Fixes a potential stall in MSE playback. Covered by web-platform-tests. Approved for 62.0b20.
Attachment #8998137 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.