Closed Bug 598870 Opened 14 years ago Closed 11 years ago

Teach video-decoder event loop to process IPC messages

Categories

(Core :: IPC, defect)

Product:
Core
Component:
IPC
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED INVALID
Tracking Flags:
Tracking Status
fennec - ---

People

(Reporter: cjones, Unassigned)

References

Details

We'll want to send and receive IPDL messages from the video decoder thread.  This means the decoder's event loop needs to be able to poll for incoming messages.  I'm not sure whose purview this falls under, so I'll just stick it under IPC for now.
Needed for faster video in Fennec.
tracking-fennec: --- → ?
Assignee: nobody → jones.chris.g
tracking-fennec: ? → 2.0b3+
Maybe blocks final if GL doesn't get our current impl playable, but doesn't block b3.
tracking-fennec: 2.0b3+ → ?
(In reply to comment #2)
> Maybe blocks final if GL doesn't get our current impl playable, but doesn't
> block b3.

in that case, 2.0-. Please re-nom if it becomes needed
tracking-fennec: ? → 2.0-
This is the last of the yucky prep work needed for DirectVideo (TM).  We need to hack around our horrible event-loop situation.  The basic problem is
 - all our IPC code uses chromium thread/polling code.  This can't be fixed in the 4.0 timeframe.
 - all the media code uses XPCOM thread/polling code.  Pretty sure there's no impetus right now to change that, nor would that that feasible in the 4.0 timeframe.
 - (presumably) the media code has to work in --disable-ipc builds

The solution is almost certainly going to require using our XPCOM-to-chromium bridge.  I'd like to hack this in with minimal collateral damage.  To do so, we're going to need to flesh out the threading model for DirectVideo a bit more.  My understanding so far is that we're going to have
 - audio+video decoder thread in content
 - "director" thread in content
 - audio playback thread in compositor

Questions
 - which content thread needs to send the audio data to the compositor?  We could send directly from the decoder, or if it's already being posted to the director, we could send from there.
 - which compositor thread needs to receive the audio data?  We could send it to the compositor main thread and have it route the data over to the playback thread, send it directly to the playback thread.
 - where are we going to do YUV->RGB conversion when we don't have GPU rendering?  Options seem to be
   * a/v decoder thread in content
   * compositor main thread
   * another thread in content or compositor

Guessing at answers there, I'm going to propose
 - have the a/v decoder post frames and audio data to the director thread in content, and have the director thread do all IPC
 - send YUV frames and audio data to the compositor main thread (so the compositor main thread does all IPC).  The compositor main thread will act as a sort of "remote director".
 - post audio data from the compositor main thread to the playback thread
 - for starters, do YUV->RGB conversion on the compositor main thread when GPU rendering isn't available.  This is banking on us having GL for fennec.
 - as a followup, if needed, do YUV->RGB conversion on another thread in content, and have that thread post converted frames to the content director thread.

How does that sound?

If that's a reasonable plan, then I think we'll only need to hack in IPC crud for the content-process director thread.  Maybe subclassing this for MOZ_IPC would be cleanest, but I'd like to discuss that.
Who uses disable-ipc? Would making MOZ_MEDIA require MOZ_IPC be that bad?
(In reply to comment #4)
> Guessing at answers there, I'm going to propose

I neglected to consider messages that might want to block on responses.  That could change the design.

(In reply to comment #5)
> Who uses disable-ipc? Would making MOZ_MEDIA require MOZ_IPC be that bad?

AFAIK, developers with puny build machines and platforms <= Tier II.  I don't think it would be the end of the world.
(In reply to comment #5)
> Who uses disable-ipc? Would making MOZ_MEDIA require MOZ_IPC be that bad?

Thunderbird uses --disable-ipc:
http://mxr.mozilla.org/comm-central/source/mail/confvars.sh#54
Oleg had started working on this.  We need to figure out the threading model.

Is there a single state-machine thread, or still one per media stream?  If there's just one, then we can have it spin an IPC-friendly event loop under the covers.  Not sure the best way to do that while integrating with the current code.

If there's more than one state-machine thread, then we'll need to make a new thread for media IPC, but just one.  The idea is to have PMedia/PVideo/whatever be a protocol opened by PContent.  See [1] for syntax.

Now, to open PMedia, the following process will need to happen
 (1) On the content-process main thread, some code needs to call PMedia::Open(contentChild).  See [2] for an example.
 (2) PMedia::Open sends a message to ContentParent, which needs to create a PMediaParent.  That channel can be opened on either the chrome-process main thread, or on another thread.  The example at [3] (OpenParent + TestOpensParent::AllocPTestOpensOpened) opens the channel on an off-main thread.
 (3) PMedia::Open also sends a "loopback" message to ContentChild, which needs to create a PMediaChild.  This channel *needs* to be opened on the media IPC thread.  See [4] for an example (OpenChild + TestOpensChild::AllocPTestOpensOpened).

After that, IPC should be all set up to talk PMedia from content-media-IPC thread and whichever thread in the chrome process.

This setup requires the state machine threads to post tasks to the IPC thread in order to send messages.

[1] http://mxr.mozilla.org/mozilla-central/source/ipc/ipdl/test/cxx/PTestOpens.ipdl#9
[2] http://mxr.mozilla.org/mozilla-central/source/ipc/ipdl/test/cxx/TestOpens.cpp#152
[3] http://mxr.mozilla.org/mozilla-central/source/ipc/ipdl/test/cxx/TestOpens.cpp#51
[4] http://mxr.mozilla.org/mozilla-central/source/ipc/ipdl/test/cxx/TestOpens.cpp#160
(In reply to Chris Jones [:cjones] [:warhammer] from comment #8)
> Is there a single state-machine thread, or still one per media stream?  If
> there's just one, then we can have it spin an IPC-friendly event loop under
> the covers.  Not sure the best way to do that while integrating with the
> current code.

There's a single one for all streams.
OK.  For now we're using a separate media/IPC thread anyway, so switching the state-machine thread event loop isn't blocking anything.

Chris (P.), we should probably try and chat for a few minutes next week about the switch.
Actually I've made minor fix which is processing decoding in MediaBridge thread, that seems to work fine.
Please don't do that.  That removes parallelism in decoding multiple media streams.  It also means that sending frames from video V1 to the chrome process can be blocked on the decode of a frame for video V2.
Based on 598868 comment 20, I think the work here now is just to swap in an IPC-compatible event loop underneath the state machine thread.  That's just a search 'n replace job.
In more detail, basically
 s/nsThread/Thread/
 s/nsRunnable/Task/
 s/NS_DispatchToCurrentThread/MessageLoop::current()->PostTask/

and that should just about do it after cleaning up syntax errors.
blocking-kilimanjaro: --- → +
How does this relate to the OMTV decoding work that just landed?
That work adds a dedicated thread for IPC, which we don't strictly need.

However because of the synchronous proxies we had to add, we probably now rely on having a separate thread that doesn't block on any thread for correctness (to avoid deadlocking).  This work definitely isn't a blocker anymore.
blocking-basecamp: + → ---
blocking-kilimanjaro: + → ---
Assignee: jones.chris.g → nobody
This bug is obsolete now.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.