Another browser thread for media (video, audio)?

[Anton Vayvod]

Hi,

I need a thread separate from all that BrowserThread has now to setup a webcam and capture video frames from it.

I know I can subclass base::Thread for that but heard some rumors about adding some kind of media thread to BrowserThread for video/audio decoding so I thought I should ask here first.

If there should be such a global thread, could I add it and whom to send the code for review? (AFAIK it only takes one more enum member and a couple of variables and methods in BrowserProcess).

Thanks,

Anton.

[Andrew Scherkus]

+hclam

As far as I know we haven't added any specific media threads in the browser process.  We use the IO thread for handling audio playback:

http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/audio_renderer_host.cc

In the renderer we have a lot of threads dedicated to media playback and decoding, but they're just normal threads.

Andrew

[Alpha Lam]

We didn't define a ChromeThread specifically for audio. But all audio output devices share the same instance of AudioManager which has a thread (http://src.chromium.org/viewvc/chrome/trunk/src/media/audio/audio_manager_base.cc). You can check in code into media/video that does a similar thing. I'll be happy to review that.

Alpha

2010年10月14日10:40 Andrew Scherkus <sche...@chromium.org>:

[Sergey Ulanov]

We do have audio thread in the browser process (see src/media/audio/audio_manager_base.cc), but it is not in BrowserThread. It is used for all audio IO.

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Anton Vayvod]

Thanks, my code doesn't belong to media/video so I think I'll go with using my own thread for now.

Or do you think it's better to have something common between my camera code and code in media/video?

[Andrew Scherkus]

It might be nice to put some generic components for webcam initialization + reading in media/camera so it can be reused (say for when HTML webcam stuff rolls around), but overall I'm indifferent.

Andrew

[John Abd-El-Malek]

If you're posting tasks back and forth from the media thread to another thread (i.e. IO), then it would be safer to add that thread to BrowserThread (what used to be ChromeThread).  That way you ensure that you don't use an invalid MessageLoop pointer on shutdown.

[Darin Fisher]

Or use MessageLoopProxy.

-Darin

[Sanjeev Radhakrishnan]

Extending that (and on a slightly tangential note), if you fund yourself holding on to MessageLoop pointers for posting tasks, please consider using a MessageLoopProxy instead. It is refcounted and can outlive the actual message loop. If the associated message loop does not exist any longer the PostTask* methods will return false (similar to the BrowserThread methods).

[Anton Vayvod]

I need to post tasks between UI and my thread. For UI thread I use BrowserThread. I used it for posting camera tasks to IO thread.

Now for my thread I use its message_loop() checking if it's not NULL. I don't store any pointers to it. The thread object is a member of the ref-counted object that's used in tasks I post as |this| pointer.

Does it seem okay or am I missing something?

That said, the simplest way for me would be to add BrowserThread::VIDEO (at least under #ifdef OS_CHROMEOS) but I'm not convinced that it's necessary to create one more global thread which is used only in one specific place until media/video will want to use it too.

[John Abd-El-Malek]

The specific scenario you describe below is safe.  BrowerThread (and later, MessageLoopProxy) were created to make it always safe, independent of which threads you hop between, so that people don't have to think before they post a task.

If your thread is only used in a few places (i.e. there isn't a lot of random code that is posting tasks to it like we do for file thread for example), then it doesn't need to be put in BrowserThread, which is a sort of global repository for threads's loops.  You can use MessageLoopProxy.

[William Chan (陈智昌)]

My take is that BrowserThread is a set of globals.  Use it if you need your thread to have global visibility.  Also, note that this makes your code a bigger pain to move out of chrome/browser/ if you foresee a possibility of that happening in the future.  MessageLoopProxy makes it more general.  But sometimes it's nice to code in absolutes (ie I want the chrome browser IO thread, not some IO like thread with a MessageLoopForIO).  BrowserThread is good for this.  Oh, and like all globals, you get to avoid passing around a kludgy parameter.  But then again, it has the same problem like other globals (hidden dependencies, requires synchronization, etc).

In your case, I think MessageLoopProxy is the way to go.

On Thu, Oct 14, 2010 at 12:32 PM, Anton Vayvod <ava...@chromium.org> wrote:

[Satish Sampath]

Anton, if you are adding webcam capture code for Chrome OS and the general design would be useful for other platforms as well, please consider placing it in media/video so we could extend or use that as is for other features. (perhaps this also includes some code which may work in Chromium Linux). I can see the video capture part of HTML <device> element (to be implemented soon) making use of this code if possible.

--

Cheers

Satish

[Andrew Scherkus]

Yeah I'd suggest /media/camera as /media/video is more to do with decoding and video-esque things

Take a look at how /media/audio is set up as some regular reusable components, which are then used by /chrome/browser and various /media/tools etc...

Andrew

[Anton Vayvod]

Anyone from threading gurus willing to perform code review about adding thread for video capturing?

There's UserImageScreen class that has scoped_refptr on Camera instance.

Camera has base::Thread member that accepts all the tasks as Camera methods.

There's an argument on how the thread should be stopped so that it doesn't outlive the UserImageScreen instance.

See http://codereview.chromium.org/3874002/show, please.
Thanks in advance!