Closed
Bug 1178691
Opened 10 years ago
Closed 9 years ago
AddOutputStream and RecreateDecodedStream run cross-thread
Categories
(Core :: Audio/Video: Playback, defect, P2)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
DUPLICATE
of bug 1178718
People
(Reporter: bholley, Unassigned)
References
Details
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•10 years ago
|
Flags: needinfo?(jwwang)
Comment 1•10 years ago
|
||
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•10 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.
Comment 3•10 years ago
|
||
I see. I will work on that now.
Comment 4•10 years ago
|
||
Let's see if bug 1178718 fix the problem.
Comment 5•10 years ago
|
||
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()?
Updated•9 years ago
|
Component: Audio/Video → Audio/Video: Playback
Updated•9 years ago
|
Priority: -- → P2
Comment 6•9 years ago
|
||
Bug 1178718 has done the job.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•