Position overlays in AVDACodecImage

media/gpu/avda_shared_state.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/gpu/avda_shared_state.h"
 
 #include "base/metrics/histogram_macros.h"
 #include "base/time/time.h"
 #include "media/gpu/avda_codec_image.h"
 #include "ui/gl/android/surface_texture.h"
@@ skipping 43 matching lines @@
 
   // Protects changes to listener_.
   base::Lock lock_;
 
   // The AVDASharedState that wants the OnFrameAvailable callback.
   AVDASharedState* listener_;
 
   DISALLOW_COPY_AND_ASSIGN(OnFrameAvailableHandler);
 };
 
-AVDASharedState::AVDASharedState()
+AVDASharedState::AVDASharedState(
+    scoped_refptr<AVDASurfaceBundle> surface_bundle)
     : frame_available_event_(base::WaitableEvent::ResetPolicy::AUTOMATIC,
                              base::WaitableEvent::InitialState::NOT_SIGNALED),
 
       gl_matrix_{
           1, 0, 0, 0,  // Default to a sane guess just in case we can't get the
           0, 1, 0, 0,  // matrix on the first call. Will be Y-flipped later.
           0, 0, 1, 0,  //
           0, 0, 0, 1,  // Comment preserves 4x4 formatting.
-      } {}
+      },
+      surface_bundle_(surface_bundle),
+      weak_this_factory_(this) {
+  // If the surface bundle has a surface texture, then register to receive
+  // OnFrameAvailable notifications.
+  if (surface_texture()) {
+    on_frame_available_handler_ =

[tguilbert, 2017/05/16 20:41:59]

Curiosity: Do you know why this is a scoped ptr and not a unique ptr?

[liberato (no reviews please), 2017/05/16 20:59:53]

both |this| and the callback own a reference to it.  |this| uses the reference
to be able to turn it off later.

one might be able to do something with a wp to the shared state, but would be
tricky since the callbacks happen on random threads.

+        new OnFrameAvailableHandler(this, surface_texture());
+  }
+
+  // If we're holding a reference to an overlay, then register to drop it if the
+  // overlay's surface is destroyed.
+  if (surface_bundle_ && surface_bundle_->overlay) {

[watk, 2017/05/17 19:45:14]

surface_bundle_ can't be null here right?

if (overlay()) {
  overlay()->AddSurfaceDestro...

[liberato (no reviews please), 2017/05/17 20:19:33]

good point, done.

+    surface_bundle_->overlay->AddSurfaceDestroyedCallback(base::Bind(
+        &AVDASharedState::OnSurfaceDestroyed, weak_this_factory_.GetWeakPtr()));
+  }
+}
 
 AVDASharedState::~AVDASharedState() {
-  if (!surface_texture_)
+  if (!surface_texture())

[watk, 2017/05/17 19:45:14]

this is bizarre looking, haha

We should have written:

if (on_frame_available_handler_)
  on_frame_available_handler_->ClearListener();

[liberato (no reviews please), 2017/05/17 20:19:33]

good point!

     return;
 
   on_frame_available_handler_->ClearListener();
 }
 
 void AVDASharedState::SignalFrameAvailable() {
   frame_available_event_.Signal();
 }
 
 void AVDASharedState::WaitForFrameAvailable() {
@@ skipping 19 matching lines @@
   SCOPED_UMA_HISTOGRAM_TIMER("Media.AvdaCodecImage.WaitTimeForFrame");
   if (!frame_available_event_.TimedWait(remaining)) {
     DVLOG(1) << "WaitForFrameAvailable() timed out, elapsed: "
              << elapsed.InMillisecondsF()
              << "ms, additionally waited: " << remaining.InMillisecondsF()
              << "ms, total: " << (elapsed + remaining).InMillisecondsF()
              << "ms";
   }
 }
 
-void AVDASharedState::SetSurfaceTexture(
-    scoped_refptr<SurfaceTextureGLOwner> surface_texture) {
-  DCHECK(surface_texture);
-  surface_texture_ = surface_texture;
-  on_frame_available_handler_ =
-      new OnFrameAvailableHandler(this, surface_texture_.get());
-}
-
 void AVDASharedState::RenderCodecBufferToSurfaceTexture(
     MediaCodecBridge* codec,
     int codec_buffer_index) {
   if (!release_time_.is_null())
     WaitForFrameAvailable();
   codec->ReleaseOutputBuffer(codec_buffer_index, true);
   release_time_ = base::TimeTicks::Now();
 }
 
 void AVDASharedState::UpdateTexImage() {
-  surface_texture_->UpdateTexImage();
+  surface_texture()->UpdateTexImage();
   // Helpfully, this is already column major.
-  surface_texture_->GetTransformMatrix(gl_matrix_);
+  surface_texture()->GetTransformMatrix(gl_matrix_);
 }
 
 void AVDASharedState::GetTransformMatrix(float matrix[16]) const {
   memcpy(matrix, gl_matrix_, sizeof(gl_matrix_));
 }
 
+void AVDASharedState::OnSurfaceDestroyed(AndroidOverlay* overlay) {
+  if (surface_bundle_ && surface_bundle_->overlay.get() == overlay)

[watk, 2017/05/17 19:45:14]

if (overlay() == overlay)?

[liberato (no reviews please), 2017/05/17 20:19:32]

oooohhhhh.  done.  yes, i originally just compared 'surface_bundle_->overlay ==
overlay'.

+    surface_bundle_ = nullptr;
+}
+
 }  // namespace media