Upstream implementation for embedded video for WebMediaPlayerAndroid

webkit/media/android/webmediaplayer_android.cc

Index: webkit/media/android/webmediaplayer_android.cc
diff --git a/webkit/media/android/webmediaplayer_android.cc b/webkit/media/android/webmediaplayer_android.cc
index b4184dadadf9ce8f67da5840da276baabb343e96..48482f28360f7bbebe1850f41d2c76e96db25df5 100644
--- a/webkit/media/android/webmediaplayer_android.cc
+++ b/webkit/media/android/webmediaplayer_android.cc
@@ -17,6 +17,8 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebCookieJar.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebRect.h"
+#include "webkit/media/android/stream_texture_factory_android.h"
+#include "webkit/media/android/webmediaplayer_manager_android.h"
 #include "webkit/media/android/webmediaplayer_proxy_android.h"
 #include "webkit/media/webmediaplayer_util.h"
 #include "webkit/media/webvideoframe_impl.h"
@@ -33,6 +35,9 @@ using media::MediaPlayerBridge;
 using media::VideoFrame;
 using webkit_media::WebVideoFrameImpl;
 
+// TODO(qinmin): Figure out where we should define this more appropriately
+#define GL_TEXTURE_EXTERNAL_OES 0x8D65
+
 namespace webkit_media {
 
 // Because we create the media player lazily on android, the duration of the
@@ -48,7 +53,9 @@ bool WebMediaPlayerAndroid::incognito_mode_ = false;
 
 WebMediaPlayerAndroid::WebMediaPlayerAndroid(
     WebMediaPlayerClient* client,
-    WebKit::WebCookieJar* cookie_jar)
+    WebKit::WebCookieJar* cookie_jar,
+    webkit_media::WebMediaPlayerManagerAndroid* manager,
+    webkit_media::StreamTextureFactory* factory)
     : client_(client),
       buffered_(1u),
       video_frame_(new WebVideoFrameImpl(VideoFrame::CreateEmptyFrame())),
@@ -61,16 +68,27 @@ WebMediaPlayerAndroid::WebMediaPlayerAndroid(
       playback_completed_(false),
       buffered_bytes_(0),
       cookie_jar_(cookie_jar),
+      manager_(manager),
       pending_play_event_(false),
       network_state_(WebMediaPlayer::NetworkStateEmpty),
-      ready_state_(WebMediaPlayer::ReadyStateHaveNothing) {
-  video_frame_.reset(new WebVideoFrameImpl(VideoFrame::CreateEmptyFrame()));
+      ready_state_(WebMediaPlayer::ReadyStateHaveNothing),
+      texture_id_(0),
+      stream_id_(0),
+      needs_establish_peer_(true),
+      stream_texture_proxy_initialized_(false),
+      stream_texture_factory_(factory) {
+  player_id_ = manager_->RegisterMediaPlayer(this);
+  if (stream_texture_factory_.get())
+    stream_texture_proxy_.reset(stream_texture_factory_->CreateProxy());
 }
 
 WebMediaPlayerAndroid::~WebMediaPlayerAndroid() {
   if (media_player_.get()) {
     media_player_->Stop();
+    media_player_->SetVideoSurface(0);
   }
+
+  manager_->UnRegisterMediaPlayer(player_id_);
 }
 
 void WebMediaPlayerAndroid::InitIncognito(bool incognito_mode) {
@@ -429,6 +447,29 @@ void WebMediaPlayerAndroid::UpdateReadyState(
   client_->readyStateChanged();
 }
 
+void WebMediaPlayerAndroid::ReleaseMediaResources() {
+  // Pause the media player first.
+  pause();
+  client_->playbackStateChanged();
+
+  if (media_player_.get()) {
+    // Save the current media player status.
+    pending_seek_ = currentTime();
+    duration_ = duration();
+
+    // Disconnecting the Surface(Texture) from the producer side allows

[scherkus (not reviewing), 2012/05/23 22:45:46]

nit on consistent terminology: what's Surface(Texture)? do you mean surface,
texture, or surface texture?

[qinmin, 2012/05/24 20:30:28]

Surface is wrapper of surfaceTexture. I remember sometime for honeycomb,
mediaplayer->setSurfaceTexture() is called. But now everything switched to
surface.

Just changed it to Surface to be consistent.

On 2012/05/23 22:45:46, scherkus wrote:

+    // all buffers but the one current (to the client) to be freed.
+    // We intentionally keep the current texture so we have something
+    // to show in the video layer.
+    // At the same time we happily avoid recreating the SurfaceTexture to
+    // not have to deal with rewiring any proxies receiving notifications.
+    media_player_->SetVideoSurface(0);

[scherkus (not reviewing), 2012/05/23 22:45:46]

what does 0 represent here?

[qinmin, 2012/05/24 20:30:28]

NULL

On 2012/05/23 22:45:46, scherkus wrote:

+    media_player_.reset(NULL);
+    needs_establish_peer_ = true;

[scherkus (not reviewing), 2012/05/23 22:45:46]

I'm not sure I follow what's happening here

in PlayInternal() we establish a peer and set needs_establish_peer_ to false

what code is forcing us to reestablish a peer?

[qinmin, 2012/05/24 20:30:28]

When calling SetVideoSurface(0), we are losing the surface peer. So setting it
to true will make us call establishPeer() next time when PlayInternal() is
called.

On 2012/05/23 22:45:46, scherkus wrote:

+  }
+  prepared_ = false;
+}
+
 void WebMediaPlayerAndroid::SetVideoSurface(jobject j_surface) {
   if (media_player_.get())
     media_player_->SetVideoSurface(j_surface);
@@ -458,6 +499,16 @@ void WebMediaPlayerAndroid::InitializeMediaPlayer() {
 void WebMediaPlayerAndroid::PlayInternal() {
   CHECK(prepared_);
 
+  if (hasVideo() && stream_texture_factory_.get()) {
+    if (!stream_id_)
+      CreateStreamTexture();
+
+    if (needs_establish_peer_) {
+      stream_texture_factory_->EstablishPeer(stream_id_, player_id_);
+      needs_establish_peer_ = false;
+    }
+  }
+
   if (paused())
     media_player_->Start(base::Bind(
         &WebMediaPlayerProxyAndroid::PlaybackCompleteCallback, proxy_));
@@ -475,7 +526,35 @@ void WebMediaPlayerAndroid::SeekInternal(float seconds) {
       &WebMediaPlayerProxyAndroid::SeekCompleteCallback, proxy_));
 }
 
+void WebMediaPlayerAndroid::CreateStreamTexture() {
+  DCHECK(!stream_id_ && !texture_id_);

[scherkus (not reviewing), 2012/05/23 22:45:46]

nit: break into 2 DCHECKs

[qinmin, 2012/05/24 20:30:28]

Done.

+  stream_id_ = stream_texture_factory_->CreateStreamTexture(&texture_id_);
+  if (texture_id_)

[scherkus (not reviewing), 2012/05/23 22:45:46]

when would this happen?

[qinmin, 2012/05/24 20:30:28]

For DRT tests, we currently don't allocate any texture thing. So no image diff
for all the media tests.

On 2012/05/23 22:45:46, scherkus wrote:

[scherkus (not reviewing), 2012/05/24 21:36:32]

That seems unfortunate! Do we have DRT image diff tests at all or is it only
media?

[qinmin, 2012/05/25 01:04:20]

I think we are currently turning all image diff off due to the settings in our
image decoders.

On 2012/05/24 21:36:32, scherkus wrote:

+    video_frame_.reset(new WebVideoFrameImpl(VideoFrame::WrapNativeTexture(
+        texture_id_,
+        GL_TEXTURE_EXTERNAL_OES,
+        texture_size_.width,
+        texture_size_.height,
+        base::TimeDelta(),
+        base::TimeDelta(),
+        base::Bind(&WebMediaPlayerAndroid::DestroyStreamTexture,

[scherkus (not reviewing), 2012/05/23 22:45:46]

do you know which thread this will eventually get called on?

which threads are calling CreateStreamTexture/DestroyStreamTexture?

[qinmin, 2012/05/24 20:30:28]

Both will be called on the main render thread.

On 2012/05/23 22:45:46, scherkus wrote:

+                   base::Unretained(this)))));
+}
+
+void WebMediaPlayerAndroid::DestroyStreamTexture() {
+  DCHECK(texture_id_ && stream_id_);

[scherkus (not reviewing), 2012/05/23 22:45:46]

nit: break into 2 DCHECKs

[qinmin, 2012/05/24 20:30:28]

Done.

+  stream_texture_factory_->DestroyStreamTexture(texture_id_);
+  texture_id_ = 0;
+  stream_id_ = 0;
+}
+
 WebVideoFrame* WebMediaPlayerAndroid::getCurrentFrame() {

[scherkus (not reviewing), 2012/05/23 22:45:46]

which thread is this getting called on?

[qinmin, 2012/05/24 20:30:28]

This is called on the compositor thread.

On 2012/05/23 22:45:46, scherkus wrote:

+  if (!stream_texture_proxy_initialized_ && stream_id_) {
+    stream_texture_proxy_->Initialize(

[scherkus (not reviewing), 2012/05/23 22:45:46]

this code assumes that CreateStreamTexture() has been called first?

could we move the proxy_ initialization code there instead of relying on
getCurrentFrame()?

[qinmin, 2012/05/24 20:30:28]

The stream_texture_proxy has to be created on the main thread, as it is
listening to the frame producer on that thread. 
And the initialize function has to be called on the compositor thread, as this
is where the frame available messages will be sent to.
The class has locks inside it to guarantee thread safety.

On 2012/05/23 22:45:46, scherkus wrote:

+        stream_id_, video_frame_->width(), video_frame_->height());
+    stream_texture_proxy_initialized_ = true;
+  }
+
   return video_frame_.get();
 }
 
@@ -483,4 +562,11 @@ void WebMediaPlayerAndroid::putCurrentFrame(
     WebVideoFrame* web_video_frame) {
 }
 
+// This gets called both on impl and main thread.

[scherkus (not reviewing), 2012/05/23 22:45:46]

what's the impl thread?

is this function racy?

[qinmin, 2012/05/24 20:30:28]

Impl thread is the compositor thread.
This should not be racy based on how compositor thread is implemented. And when
calling this on the main thread, we are passing a null params to the function to
clean it up. 


On 2012/05/23 22:45:46, scherkus wrote:

+void WebMediaPlayerAndroid::setStreamTextureClient(
+    WebKit::WebStreamTextureClient* client) {
+  if (stream_texture_proxy_.get())
+    stream_texture_proxy_->SetClient(client);
+}
+
 }  // namespace webkit_media