player_x11 : change X/GL thread to message loop for injecting task

change X/GL thread to message loop for injecting task

1. there are need to allocate buffer in this thread, requested by other threads. therefore it is natural to use message loop instead of infinite loop.
2. in the future, gpu command buffer consumer is running on it own thread and own the eglContext. we need to post buffer allocation there, therefore this change make player_x11 more resemble the real workflow in chrome.

TEST= glVideoRender on host machine.
glesVideoRender/x11VideoRender on arm netbook.

BUGS=None

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=49255

[jiesun, 2010-06-08 20:28:47]

please review.

[Alpha Left Google, 2010-06-08 22:36:52]

MediaFilter already provides MessageLoop, is that not enough?

http://codereview.chromium.org/2724005/diff/2001/3001
File media/tools/player_x11/gl_video_renderer.cc (right):

http://codereview.chromium.org/2724005/diff/2001/3001#newcode266
media/tools/player_x11/gl_video_renderer.cc:266: if ( message_loop_ ) {
nit: if (message_loop())

http://codereview.chromium.org/2724005/diff/2001/3002
File media/tools/player_x11/gl_video_renderer.h (right):

http://codereview.chromium.org/2724005/diff/2001/3002#newcode35
media/tools/player_x11/gl_video_renderer.h:35: void
set_message_loop(MessageLoop* message_loop) {
MediaFilter class already provides set_message_loop() and message_loop() so not
need to add setter and member here.

http://codereview.chromium.org/2724005/diff/2001/3003
File media/tools/player_x11/gles_video_renderer.cc (right):

http://codereview.chromium.org/2724005/diff/2001/3003#newcode152
media/tools/player_x11/gles_video_renderer.cc:152: if ( message_loop_ ) {
nit: if (message_loop)

http://codereview.chromium.org/2724005/diff/2001/3004
File media/tools/player_x11/gles_video_renderer.h (right):

http://codereview.chromium.org/2724005/diff/2001/3004#newcode38
media/tools/player_x11/gles_video_renderer.h:38: void
set_message_loop(MessageLoop* message_loop) {
MediaFilter class already provides set_message_loop() and message_loop() so not
need to add setter and member here.

http://codereview.chromium.org/2724005/diff/2001/3006
File media/tools/player_x11/x11_video_renderer.cc (right):

http://codereview.chromium.org/2724005/diff/2001/3006#newcode135
media/tools/player_x11/x11_video_renderer.cc:135: if ( message_loop_ ) {
nit: if (message_loop())

http://codereview.chromium.org/2724005/diff/2001/3007
File media/tools/player_x11/x11_video_renderer.h (right):

http://codereview.chromium.org/2724005/diff/2001/3007#newcode34
media/tools/player_x11/x11_video_renderer.h:34: void
set_message_loop(MessageLoop* message_loop) {
MediaFilter class already provides set_message_loop() and message_loop() so not
need to add setter and member here.

[jiesun, 2010-06-08 23:28:21]

please have another look!

[Alpha Left Google, 2010-06-08 23:42:38]

One lat nit:

Should update if ( glx_thread_message_loop() ) to if
(glx_thread_message_loop()). After that LGTM.

[wjia(left Chromium), 2010-06-09 01:08:05]

LGTM except the comment on message_loop.

http://codereview.chromium.org/2724005/diff/14001/15004
File media/tools/player_x11/gles_video_renderer.h (right):

http://codereview.chromium.org/2724005/diff/14001/15004#newcode40
media/tools/player_x11/gles_video_renderer.h:40: }
Is it possible to use the message_loop in MediaFilter?

[jiesun, 2010-06-09 01:18:36]

http://codereview.chromium.org/2724005/diff/14001/15004
File media/tools/player_x11/gles_video_renderer.h (right):

http://codereview.chromium.org/2724005/diff/14001/15004#newcode42
media/tools/player_x11/gles_video_renderer.h:42: MessageLoop*
glx_thread_message_loop() {
MediaFilter::message_loop_ mostly mean its own message loop on it own thread,
not someone else's message loop.
If in the future, we need a message loop based VideoRenderBase, then this
overload will be troublesome and confusing. thanks