WebM buffered broken with cloned decoders

RESOLVED FIXED

Status

()

Core
Audio/Video
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: kinetik, Assigned: kinetik)

Tracking

(Blocks: 1 bug)

Trunk
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
The offset-to-time mapping for WebM's buffered implementation is built as the data is received on a stream and stored on the decoder's reader.  When a decoder is cloned, this information is not shared, but the underlying cache data is--this means that the cloned decoder does not have an accurate offset-to-time mapping.

Fix is fairly simple--share the offset-to-time mapping and related machinery between cloned decoders.
(Assignee)

Comment 1

8 years ago
Created attachment 476987 [details] [diff] [review]
patch v0

Move the offset-to-time mapping and parser state into a refcounted class and create or share an instance depending on whether we're creating a new decoder or cloning an existing one.

I'm not sure if passing the original decoder down via Load is the cleanest way to do this.  The decoder's reader isn't created until nsMediaDecoder::Load is called, so it's not possible to share the state when nsMediaDecoder::Clone is called.  Another approach would be to move the nsWebMBufferedState up to the nsWebMDecoder and change nsMediaDecoder::Clone to pass in the original decoder, allowing this particular piece of state to be shared earlier (but doesn't work for any other case where a state machine or reader created in Load would want to share data between decoders).

Working on a test for this now, but requesting review to get early feedback.
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Attachment #476987 - Flags: review?(roc)
Comment on attachment 476987 [details] [diff] [review]
patch v0

Looks good.
Attachment #476987 - Flags: review?(roc) → review+
(Assignee)

Comment 3

8 years ago
Oops, thought I'd requested blocking on this already.  It should block because it breaks buffered and the built-in controls when fullscreening a video.
blocking2.0: --- → ?
(Assignee)

Updated

8 years ago
Flags: in-testsuite?
Blocks: 598242
blocking2.0: ? → final+
Whiteboard: [needs landing]
(Assignee)

Updated

8 years ago
Blocks: 600724
http://hg.mozilla.org/mozilla-central/rev/bc87d90b82dc
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
You need to log in before you can comment on or make changes to this bug.