Begin refactor of AVDA to support zero-copy.

content/common/gpu/media/android_video_decode_accelerator_base.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.

[watk, 2015/08/28 21:52:05]

s/2013/2015

[liberato (no reviews please), 2015/09/04 17:59:47]

Done.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/common/gpu/media/android_video_decode_accelerator.h"
+#include "content/common/gpu/media/android_video_decode_accelerator_base.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "content/common/gpu/gpu_channel.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 #include "media/base/bitstream_buffer.h"
 #include "media/base/limits.h"
 #include "media/base/video_decoder_config.h"
 #include "media/video/picture.h"
 #include "ui/gl/android/scoped_java_surface.h"
 #include "ui/gl/android/surface_texture.h"
 #include "ui/gl/gl_bindings.h"
 
 namespace content {
 
 // Helper macros for dealing with failure.  If |result| evaluates false, emit
 // |log| to ERROR, register |error| with the decoder, and return.
-#define RETURN_ON_FAILURE(result, log, error)                     \
-  do {                                                            \
-    if (!(result)) {                                              \
-      DLOG(ERROR) << log;                                         \
-      base::MessageLoop::current()->PostTask(                     \
-          FROM_HERE,                                              \
-          base::Bind(&AndroidVideoDecodeAccelerator::NotifyError, \
-                     weak_this_factory_.GetWeakPtr(),             \
-                     error));                                     \
-      state_ = ERROR;                                             \
-      return;                                                     \
-    }                                                             \
+#define RETURN_ON_FAILURE(result, log, error)                         \
+  do {                                                                \
+    if (!(result)) {                                                  \
+      DLOG(ERROR) << log;                                             \
+      base::MessageLoop::current()->PostTask(                         \
+          FROM_HERE,                                                  \
+          base::Bind(&AndroidVideoDecodeAcceleratorBase::NotifyError, \
+                     weak_this_factory_.GetWeakPtr(),                 \
+                     error));                                         \
+      state_ = ERROR;                                                 \
+      return;                                                         \
+    }                                                                 \
   } while (0)
 
-// TODO(dwkang): We only need kMaxVideoFrames to pass media stack's prerolling
-// phase, but 1 is added due to crbug.com/176036. This should be tuned when we
-// have actual use case.
-enum { kNumPictureBuffers = media::limits::kMaxVideoFrames + 1 };
-
 // Max number of bitstreams notified to the client with
 // NotifyEndOfBitstreamBuffer() before getting output from the bitstream.
 enum { kMaxBitstreamsNotifiedInAdvance = 32 };
 
 #if defined(ENABLE_MEDIA_PIPELINE_ON_ANDROID)
 // MediaCodec is only guaranteed to support baseline, but some devices may
 // support others. Advertise support for all H264 profiles and let the
 // MediaCodec fail when decoding if it's not actually supported. It's assumed
 // that consumers won't have software fallback for H264 on Android anyway.
 static const media::VideoCodecProfile kSupportedH264Profiles[] = {
@@ skipping 26 matching lines @@
   // pictures have been fed to saturate any internal buffering).  This is
   // speculative and it's unclear that this would be a win (nor that there's a
   // reasonably device-agnostic way to fill in the "believes" above).
   return base::TimeDelta::FromMilliseconds(10);
 }
 
 static inline const base::TimeDelta NoWaitTimeOut() {
   return base::TimeDelta::FromMicroseconds(0);
 }
 
-AndroidVideoDecodeAccelerator::AndroidVideoDecodeAccelerator(
+AndroidVideoDecodeAcceleratorBase::AndroidVideoDecodeAcceleratorBase(
     const base::WeakPtr<gpu::gles2::GLES2Decoder> decoder,
     const base::Callback<bool(void)>& make_context_current)
     : client_(NULL),
       make_context_current_(make_context_current),
       codec_(media::kCodecH264),
       state_(NO_ERROR),
       surface_texture_id_(0),
       picturebuffers_requested_(false),
       gl_decoder_(decoder),
       weak_this_factory_(this) {}
 
-AndroidVideoDecodeAccelerator::~AndroidVideoDecodeAccelerator() {
+AndroidVideoDecodeAcceleratorBase::~AndroidVideoDecodeAcceleratorBase() {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
-bool AndroidVideoDecodeAccelerator::Initialize(media::VideoCodecProfile profile,
-                                               Client* client) {
+bool AndroidVideoDecodeAcceleratorBase::Initialize(
+    media::VideoCodecProfile profile,
+    Client* client) {
   DCHECK(!media_codec_);
   DCHECK(thread_checker_.CalledOnValidThread());
 
   client_ = client;
   codec_ = VideoCodecProfileToVideoCodec(profile);
 
   bool profile_supported = codec_ == media::kCodecVP8;
 #if defined(ENABLE_MEDIA_PIPELINE_ON_ANDROID)
   profile_supported |=
       (codec_ == media::kCodecVP9 || codec_ == media::kCodecH264);
@@ skipping 39 matching lines @@
   surface_texture_ = gfx::SurfaceTexture::Create(surface_texture_id_);
 
   if (!ConfigureMediaCodec()) {
     LOG(ERROR) << "Failed to create MediaCodec instance.";
     return false;
   }
 
   return true;
 }
 
-void AndroidVideoDecodeAccelerator::DoIOTask() {
+void AndroidVideoDecodeAcceleratorBase::DoIOTask() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (state_ == ERROR) {
     return;
   }
 
   QueueInput();
   DequeueOutput();
 }
 
-void AndroidVideoDecodeAccelerator::QueueInput() {
+void AndroidVideoDecodeAcceleratorBase::QueueInput() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (bitstreams_notified_in_advance_.size() > kMaxBitstreamsNotifiedInAdvance)
     return;
   if (pending_bitstream_buffers_.empty())
     return;
 
   int input_buf_index = 0;
   media::MediaCodecStatus status = media_codec_->DequeueInputBuffer(
       NoWaitTimeOut(), &input_buf_index);
   if (status != media::MEDIA_CODEC_OK) {
@@ skipping 38 matching lines @@
 
   // We should call NotifyEndOfBitstreamBuffer(), when no more decoded output
   // will be returned from the bitstream buffer. However, MediaCodec API is
   // not enough to guarantee it.
   // So, here, we calls NotifyEndOfBitstreamBuffer() in advance in order to
   // keep getting more bitstreams from the client, and throttle them by using
   // |bitstreams_notified_in_advance_|.
   // TODO(dwkang): check if there is a way to remove this workaround.
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
-      base::Bind(&AndroidVideoDecodeAccelerator::NotifyEndOfBitstreamBuffer,
+      base::Bind(&AndroidVideoDecodeAcceleratorBase::NotifyEndOfBitstreamBuffer,
                  weak_this_factory_.GetWeakPtr(),
                  bitstream_buffer.id()));
   bitstreams_notified_in_advance_.push_back(bitstream_buffer.id());
 }
 
-void AndroidVideoDecodeAccelerator::DequeueOutput() {
+void AndroidVideoDecodeAcceleratorBase::DequeueOutput() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (picturebuffers_requested_ && output_picture_buffers_.empty())
     return;
 
   if (!output_picture_buffers_.empty() && free_picture_ids_.empty()) {
     // Don't have any picture buffer to send. Need to wait more.
     return;
   }
 
   bool eos = false;
@@ skipping 12 matching lines @@
 
       case media::MEDIA_CODEC_OUTPUT_FORMAT_CHANGED: {
         int32 width, height;
         media_codec_->GetOutputFormat(&width, &height);
 
         if (!picturebuffers_requested_) {
           picturebuffers_requested_ = true;
           size_ = gfx::Size(width, height);
           base::MessageLoop::current()->PostTask(
               FROM_HERE,
-              base::Bind(&AndroidVideoDecodeAccelerator::RequestPictureBuffers,
+              base::Bind(&AndroidVideoDecodeAcceleratorBase::
+                         RequestPictureBuffers,
                          weak_this_factory_.GetWeakPtr()));
         } else {
           // Dynamic resolution change support is not specified by the Android
           // platform at and before JB-MR1, so it's not possible to smoothly
           // continue playback at this point.  Instead, error out immediately,
           // expecting clients to Reset() as appropriate to avoid this.
           // b/7093648
           RETURN_ON_FAILURE(size_ == gfx::Size(width, height),
                             "Dynamic resolution change is not supported.",
                             PLATFORM_FAILURE);
         }
         return;
       }
 
       case media::MEDIA_CODEC_OUTPUT_BUFFERS_CHANGED:
         break;
 
       case media::MEDIA_CODEC_OK:
         DCHECK_GE(buf_index, 0);
         break;
 
       default:
         NOTREACHED();
         break;
     }
   } while (buf_index < 0);
 
-  // This ignores the emitted ByteBuffer and instead relies on rendering to the
-  // codec's SurfaceTexture and then copying from that texture to the client's
-  // PictureBuffer's texture.  This means that each picture's data is written
-  // three times: once to the ByteBuffer, once to the SurfaceTexture, and once
-  // to the client's texture.  It would be nicer to either:
-  // 1) Render directly to the client's texture from MediaCodec (one write); or
-  // 2) Upload the ByteBuffer to the client's texture (two writes).
-  // Unfortunately neither is possible:
-  // 1) MediaCodec's use of SurfaceTexture is a singleton, and the texture
-  //    written to can't change during the codec's lifetime.  b/11990461
-  // 2) The ByteBuffer is likely to contain the pixels in a vendor-specific,
-  //    opaque/non-standard format.  It's not possible to negotiate the decoder
-  //    to emit a specific colorspace, even using HW CSC.  b/10706245
-  // So, we live with these two extra copies per picture :(
-  media_codec_->ReleaseOutputBuffer(buf_index, true);
-
   if (eos) {
+    // TODO(liberato): Before refactoring into *Base, this was unconditionally
+    // done before the eos check, with render==true.  However, since that
+    // frame wasn't sent anywhere in the eos case, we now do it here with
+    // render==false.  We need to see if eos can actually deliver a valid
+    // frame with it.
+    media_codec_->ReleaseOutputBuffer(buf_index, false);

[watk, 2015/08/28 21:52:05]

AFAIK, this is ok and we won't get a valid frame with EOS.

[liberato (no reviews please), 2015/09/04 17:59:47]

yeah, i this so too.  i'll leave the TODO here until qinmin@ and timav@ take a
look.

     base::MessageLoop::current()->PostTask(
         FROM_HERE,
-        base::Bind(&AndroidVideoDecodeAccelerator::NotifyFlushDone,
+        base::Bind(&AndroidVideoDecodeAcceleratorBase::NotifyFlushDone,
                    weak_this_factory_.GetWeakPtr()));
   } else {
     int64 bitstream_buffer_id = timestamp.InMicroseconds();
-    SendCurrentSurfaceToClient(static_cast<int32>(bitstream_buffer_id));
+    SendCurrentSurfaceToClient(buf_index,
+        static_cast<int32>(bitstream_buffer_id));
 
     // Removes ids former or equal than the id from decoder. Note that
     // |bitstreams_notified_in_advance_| does not mean bitstream ids in decoder
     // because of frame reordering issue. We just maintain this roughly and use
     // for the throttling purpose.
     std::list<int32>::iterator it;
     for (it = bitstreams_notified_in_advance_.begin();
         it != bitstreams_notified_in_advance_.end();
         ++it) {
       if (*it == bitstream_buffer_id) {
         bitstreams_notified_in_advance_.erase(
             bitstreams_notified_in_advance_.begin(), ++it);
         break;
       }
     }
   }
 }
 
-void AndroidVideoDecodeAccelerator::SendCurrentSurfaceToClient(
+void AndroidVideoDecodeAcceleratorBase::SendCurrentSurfaceToClient(
+    int32 codec_buf_index,

[watk, 2015/08/28 21:52:05]

Slight naming inconsistency with the header: codec_buffer_index vs
codec_buf_index.

[liberato (no reviews please), 2015/09/04 17:59:47]

thanks.

     int32 bitstream_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_NE(bitstream_id, -1);
   DCHECK(!free_picture_ids_.empty());
 
   RETURN_ON_FAILURE(make_context_current_.Run(),
                     "Failed to make this decoder's GL context current.",
                     PLATFORM_FAILURE);
 
   int32 picture_buffer_id = free_picture_ids_.front();
   free_picture_ids_.pop();
 
-  float transfrom_matrix[16];
-  surface_texture_->UpdateTexImage();
-  surface_texture_->GetTransformMatrix(transfrom_matrix);
-
   OutputBufferMap::const_iterator i =
       output_picture_buffers_.find(picture_buffer_id);
   RETURN_ON_FAILURE(i != output_picture_buffers_.end(),
                     "Can't find a PictureBuffer for " << picture_buffer_id,
                     PLATFORM_FAILURE);
-  uint32 picture_buffer_texture_id = i->second.texture_id();
 
-  RETURN_ON_FAILURE(gl_decoder_.get(),
+  // Make sure that the decoder is available for AssignCurrent*().
+  RETURN_ON_FAILURE(GetGlDecoder(),
                     "Failed to get gles2 decoder instance.",
                     ILLEGAL_STATE);

[watk, 2015/08/28 21:52:05]

Should this check be done inside AssignCurrent? That it uses the gl decoder
seems like an implementation detail of AssignCurrent.

[liberato (no reviews please), 2015/09/04 17:59:47]

yeah, i went back and forth on this one.  i kept it as-is because moving the
RETURN_ON_FAILURE logic around turned out to make it ugly.

i'll see if i can figure out a way to do this better.

... i refactored RETURN_ON_FAILURE a bit and moved it into its own .h file.  now
_base and the subclasses can use it.  see if you like this better.

-  // Defer initializing the CopyTextureCHROMIUMResourceManager until it is
-  // needed because it takes 10s of milliseconds to initialize.
-  if (!copier_) {
-    copier_.reset(new gpu::CopyTextureCHROMIUMResourceManager());
-    copier_->Initialize(gl_decoder_.get());
-  }
 
-  // Here, we copy |surface_texture_id_| to the picture buffer instead of
-  // setting new texture to |surface_texture_| by calling attachToGLContext()
-  // because:
-  // 1. Once we call detachFrameGLContext(), it deletes the texture previous
-  //    attached.
-  // 2. SurfaceTexture requires us to apply a transform matrix when we show
-  //    the texture.
-  // TODO(hkuang): get the StreamTexture transform matrix in GPU process
-  // instead of using default matrix crbug.com/226218.
-  const static GLfloat default_matrix[16] = {1.0f, 0.0f, 0.0f, 0.0f,
-                                             0.0f, 1.0f, 0.0f, 0.0f,
-                                             0.0f, 0.0f, 1.0f, 0.0f,
-                                             0.0f, 0.0f, 0.0f, 1.0f};
-  copier_->DoCopyTextureWithTransform(gl_decoder_.get(),
-                                      GL_TEXTURE_EXTERNAL_OES,
-                                      surface_texture_id_,
-                                      picture_buffer_texture_id,
-                                      size_.width(),
-                                      size_.height(),
-                                      false,
-                                      false,
-                                      false,
-                                      default_matrix);
+  AssignCurrentSurfaceToPictureBuffer(codec_buf_index, i->second);
 
   // TODO(henryhsu): Pass (0, 0) as visible size will cause several test
   // cases failed. We should make sure |size_| is coded size or visible size.
   base::MessageLoop::current()->PostTask(
-      FROM_HERE, base::Bind(&AndroidVideoDecodeAccelerator::NotifyPictureReady,
-                            weak_this_factory_.GetWeakPtr(),
-                            media::Picture(picture_buffer_id, bitstream_id,
-                                           gfx::Rect(size_), false)));
+      FROM_HERE,
+      base::Bind(&AndroidVideoDecodeAcceleratorBase::NotifyPictureReady,
+                 weak_this_factory_.GetWeakPtr(),
+                 media::Picture(picture_buffer_id, bitstream_id,
+                                gfx::Rect(size_), false)));
 }
 
-void AndroidVideoDecodeAccelerator::Decode(
+void AndroidVideoDecodeAcceleratorBase::Decode(
     const media::BitstreamBuffer& bitstream_buffer) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (bitstream_buffer.id() != -1 && bitstream_buffer.size() == 0) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
-        base::Bind(&AndroidVideoDecodeAccelerator::NotifyEndOfBitstreamBuffer,
+        base::Bind(&AndroidVideoDecodeAcceleratorBase::
+                   NotifyEndOfBitstreamBuffer,
                    weak_this_factory_.GetWeakPtr(),
                    bitstream_buffer.id()));
     return;
   }
 
   pending_bitstream_buffers_.push(
       std::make_pair(bitstream_buffer, base::Time::Now()));
 
   DoIOTask();
 }
 
-void AndroidVideoDecodeAccelerator::AssignPictureBuffers(
+void AndroidVideoDecodeAcceleratorBase::RequestPictureBuffers() {
+    client_->ProvidePictureBuffers(GetNumPictureBuffers(), size_,
+        GetTextureTarget());
+}
+
+void AndroidVideoDecodeAcceleratorBase::AssignPictureBuffers(
     const std::vector<media::PictureBuffer>& buffers) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(output_picture_buffers_.empty());
   DCHECK(free_picture_ids_.empty());
 
   for (size_t i = 0; i < buffers.size(); ++i) {
     RETURN_ON_FAILURE(buffers[i].size() == size_,
                       "Invalid picture buffer size was passed.",
                       INVALID_ARGUMENT);
     int32 id = buffers[i].id();
     output_picture_buffers_.insert(std::make_pair(id, buffers[i]));
     free_picture_ids_.push(id);
     // Since the client might be re-using |picture_buffer_id| values, forget
     // about previously-dismissed IDs now.  See ReusePictureBuffer() comment
     // about "zombies" for why we maintain this set in the first place.
     dismissed_picture_ids_.erase(id);
   }
 
-  RETURN_ON_FAILURE(output_picture_buffers_.size() >= kNumPictureBuffers,
+  RETURN_ON_FAILURE(output_picture_buffers_.size() >= GetNumPictureBuffers(),
                     "Invalid picture buffers were passed.",
                     INVALID_ARGUMENT);
 
   DoIOTask();
 }
 
-void AndroidVideoDecodeAccelerator::ReusePictureBuffer(
+void AndroidVideoDecodeAcceleratorBase::ReusePictureBuffer(
     int32 picture_buffer_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // This ReusePictureBuffer() might have been in a pipe somewhere (queued in
   // IPC, or in a PostTask either at the sender or receiver) when we sent a
   // DismissPictureBuffer() for this |picture_buffer_id|.  Account for such
   // potential "zombie" IDs here.
   if (dismissed_picture_ids_.erase(picture_buffer_id))
     return;
 
   free_picture_ids_.push(picture_buffer_id);
 
   DoIOTask();
 }
 
-void AndroidVideoDecodeAccelerator::Flush() {
+void AndroidVideoDecodeAcceleratorBase::Flush() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   Decode(media::BitstreamBuffer(-1, base::SharedMemoryHandle(), 0));
 }
 
-bool AndroidVideoDecodeAccelerator::ConfigureMediaCodec() {
+bool AndroidVideoDecodeAcceleratorBase::ConfigureMediaCodec() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(surface_texture_.get());
 
   gfx::ScopedJavaSurface surface(surface_texture_.get());
 
   // Pass a dummy 320x240 canvas size and let the codec signal the real size
   // when it's known from the bitstream.
   media_codec_.reset(media::VideoCodecBridge::CreateDecoder(
       codec_, false, gfx::Size(320, 240), surface.j_surface().obj(), NULL));
   if (!media_codec_)
     return false;
 
   io_timer_.Start(FROM_HERE,
                   DecodePollDelay(),
                   this,
-                  &AndroidVideoDecodeAccelerator::DoIOTask);
+                  &AndroidVideoDecodeAcceleratorBase::DoIOTask);
   return true;
 }
 
-void AndroidVideoDecodeAccelerator::Reset() {
+void AndroidVideoDecodeAcceleratorBase::Reset() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   while (!pending_bitstream_buffers_.empty()) {
     int32 bitstream_buffer_id = pending_bitstream_buffers_.front().first.id();
     pending_bitstream_buffers_.pop();
 
     if (bitstream_buffer_id != -1) {
       base::MessageLoop::current()->PostTask(
           FROM_HERE,
-          base::Bind(&AndroidVideoDecodeAccelerator::NotifyEndOfBitstreamBuffer,
+          base::Bind(&AndroidVideoDecodeAcceleratorBase::
+                     NotifyEndOfBitstreamBuffer,
                      weak_this_factory_.GetWeakPtr(),
                      bitstream_buffer_id));
     }
   }
   bitstreams_notified_in_advance_.clear();
 
   for (OutputBufferMap::iterator it = output_picture_buffers_.begin();
        it != output_picture_buffers_.end();
        ++it) {
     client_->DismissPictureBuffer(it->first);
     dismissed_picture_ids_.insert(it->first);
   }
   output_picture_buffers_.clear();
   std::queue<int32> empty;
   std::swap(free_picture_ids_, empty);
   CHECK(free_picture_ids_.empty());
   picturebuffers_requested_ = false;
 
   // On some devices, and up to at least JB-MR1,
   // - flush() can fail after EOS (b/8125974); and
   // - mid-stream resolution change is unsupported (b/7093648).
   // To cope with these facts, we always stop & restart the codec on Reset().
   io_timer_.Stop();
   media_codec_->Stop();
   ConfigureMediaCodec();
   state_ = NO_ERROR;
 
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
-      base::Bind(&AndroidVideoDecodeAccelerator::NotifyResetDone,
+      base::Bind(&AndroidVideoDecodeAcceleratorBase::NotifyResetDone,
                  weak_this_factory_.GetWeakPtr()));
 }
 
-void AndroidVideoDecodeAccelerator::Destroy() {
+void AndroidVideoDecodeAcceleratorBase::Destroy() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   weak_this_factory_.InvalidateWeakPtrs();
   if (media_codec_) {
     io_timer_.Stop();
     media_codec_->Stop();
   }
   if (surface_texture_id_)
     glDeleteTextures(1, &surface_texture_id_);
-  if (copier_)
-    copier_->Destroy();
   delete this;
 }
 
-bool AndroidVideoDecodeAccelerator::CanDecodeOnIOThread() {
+bool AndroidVideoDecodeAcceleratorBase::CanDecodeOnIOThread() {
   return false;
 }
 
-void AndroidVideoDecodeAccelerator::RequestPictureBuffers() {
-  client_->ProvidePictureBuffers(kNumPictureBuffers, size_, GL_TEXTURE_2D);
-}
-
-void AndroidVideoDecodeAccelerator::NotifyPictureReady(
+void AndroidVideoDecodeAcceleratorBase::NotifyPictureReady(
     const media::Picture& picture) {
   client_->PictureReady(picture);
 }
 
-void AndroidVideoDecodeAccelerator::NotifyEndOfBitstreamBuffer(
+void AndroidVideoDecodeAcceleratorBase::NotifyEndOfBitstreamBuffer(
     int input_buffer_id) {
   client_->NotifyEndOfBitstreamBuffer(input_buffer_id);
 }
 
-void AndroidVideoDecodeAccelerator::NotifyFlushDone() {
+void AndroidVideoDecodeAcceleratorBase::NotifyFlushDone() {
   client_->NotifyFlushDone();
 }
 
-void AndroidVideoDecodeAccelerator::NotifyResetDone() {
+void AndroidVideoDecodeAcceleratorBase::NotifyResetDone() {
   client_->NotifyResetDone();
 }
 
-void AndroidVideoDecodeAccelerator::NotifyError(
+void AndroidVideoDecodeAcceleratorBase::NotifyError(
     media::VideoDecodeAccelerator::Error error) {
   client_->NotifyError(error);
 }
 
+media::VideoDecodeAccelerator::Client*
+AndroidVideoDecodeAcceleratorBase::GetClient() const {

[watk, 2015/08/28 21:52:05]

This and the following aren't in declaration order. The style guide suggests the
definition and declaration order should match.

[liberato (no reviews please), 2015/09/04 17:59:47]

i think that several functions were out of order before, too.  i'll move these
ones closer to where they should be, but i don't want to re-order existing ones
as part of this, in order to keep the (hopefully lack of) diff clearer.

+    return client_;
+}
+
+const gfx::Size& AndroidVideoDecodeAcceleratorBase::GetSize() const {
+    return size_;
+}
+
+const base::ThreadChecker&
+AndroidVideoDecodeAcceleratorBase::ThreadChecker() const {
+    return thread_checker_;
+}
+
+gfx::SurfaceTexture*
+AndroidVideoDecodeAcceleratorBase::GetSurfaceTexture() const {
+    return surface_texture_.get();
+}
+
+uint32 AndroidVideoDecodeAcceleratorBase::GetSurfaceTextureId() const {
+    return surface_texture_id_;
+}
+
+gpu::gles2::GLES2Decoder*
+AndroidVideoDecodeAcceleratorBase::GetGlDecoder() const {
+    return gl_decoder_.get();
+}
+
+media::VideoCodecBridge* AndroidVideoDecodeAcceleratorBase::GetMediaCodec() {
+    return media_codec_.get();
+}
+
 // static
 media::VideoDecodeAccelerator::SupportedProfiles
-AndroidVideoDecodeAccelerator::GetSupportedProfiles() {
+AndroidVideoDecodeAcceleratorBase::GetSupportedProfiles() {
   SupportedProfiles profiles;
 
   if (!media::VideoCodecBridge::IsKnownUnaccelerated(
           media::kCodecVP8, media::MEDIA_CODEC_DECODER)) {
     SupportedProfile profile;
     profile.profile = media::VP8PROFILE_ANY;
     profile.min_resolution.SetSize(0, 0);
     profile.max_resolution.SetSize(1920, 1088);
     profiles.push_back(profile);
   }
@@ skipping 17 matching lines @@
     // software fallback for H264 on Android anyway.
     profile.max_resolution.SetSize(3840, 2160);
     profiles.push_back(profile);
   }
 #endif
 
   return profiles;
 }
 
 }  // namespace content