AddOutputStream and RecreateDecodedStream run cross-thread

RESOLVED DUPLICATE of bug 1178718

Status

()

Core
Audio/Video: Playback
P2
normal
RESOLVED DUPLICATE of bug 1178718
3 years ago
2 years ago

People

(Reporter: bholley, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

3 years ago
I just finished patches to remove the last dependencies on the decoder monitor from the MDSM. I hit some assertions, and it turns out that bug 1163467 added two new methods that rely on the monitor.

I'm not too familiar with the DecodedStream stuff, but it looks like it asserts that it operates on the main thread, which makes it pretty incompatible with the MDSM's new threading model. JW, do you have suggestions on how to fix it? What would the impact be of backing out bug 1163467?

Remember: locking and mirroring don't mix. The coherency model is predicated on the assumption that there are no cross-thread reads, and we'll have races if there are.
(Reporter)

Updated

3 years ago
Flags: needinfo?(jwwang)
Which assertion are you hitting? Bug 1163467 is basically moving the stream related code to another file. DecodedStream code is kinda tricky since most MediaStream functions can only be called in the main thread which doesn't fit well into the thread model of MDSM and has been a headache to me when trying to unify AudioSink and DecodedStream.
Flags: needinfo?(jwwang)
(Reporter)

Comment 2

3 years ago
(In reply to JW Wang [:jwwang] from comment #1)
> Which assertion are you hitting?

The assertion that I'm hitting is the one in IsPlaying, which my patches added. :P

In order to get rid of the monitor, we need every method on the MDSM to assert OnTaskQueue (and same for the MediaDecoderReader). We shouldn't be adding any new code that depends on the decoder monitor for synchronization.
I see. I will work on that now.
Depends on: 1178718
Let's see if bug 1178718 fix the problem.

Updated

3 years ago
Blocks: 1156472
Try for bug 1178718 seems green.

Hi Bobby,
Can you try if the patch of bug 1178718 works for you which removes the dependency on the decoder monitor from AddOutputStream() and RecreateDecodedStream()?
Component: Audio/Video → Audio/Video: Playback
Priority: -- → P2
Bug 1178718 has done the job.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1178718
You need to log in before you can comment on or make changes to this bug.