Group AVDA output surface into AVDASurfaceBundle.

media/gpu/android_video_decode_accelerator.h

 // Copyright (c) 2013 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.
 
 #ifndef MEDIA_GPU_ANDROID_VIDEO_DECODE_ACCELERATOR_H_
 #define MEDIA_GPU_ANDROID_VIDEO_DECODE_ACCELERATOR_H_
 
 #include <stdint.h>
 
 #include <list>
@@ skipping 97 matching lines @@
     DRAIN_FOR_RESET,
     DRAIN_FOR_DESTROY,
   };
 
   // Entry point for configuring / reconfiguring a codec with a new surface.
   // Start surface creation by trying to allocate the surface id.  Will either
   // InitializePictureBufferManager if the surface is available immediately, or
   // will wait for OnSurfaceAvailable to do it.  This will transition |state_|
   // to WAITING_FOR_SURFACE or WAITING_FOR_CODEC, as needed (or NO_ERROR if it
   // gets the surface and the codec without waiting).
+  // Note that this requires that you create a new |incoming_bundle_| with the
+  // appropriate surface id.
   void StartSurfaceCreation();
 
   // Initialize of the picture buffer manager to use the current surface, once
   // it is available.  This is not normally called directly, but rather via
   // StartSurfaceCreation.  If we have a media codec already, then this will
   // attempt to setSurface the new surface.  Otherwise, it will start codec
   // config using the new surface.  In that case, there might not be a codec
   // ready even if this succeeds, but async config will be started.  If
   // setSurface fails, this will not replace the codec.  On failure, this will
   // transition |state_| to ERROR.
+  // Note that this assumes that there is an |incoming_bundle_| that we'll use.
+  // On success, we'll replace the bundle in |codec_config_|.  On failure, we'll
+  // delete the incoming bundle.
   void InitializePictureBufferManager();
 
   // A part of destruction process that is sometimes postponed after the drain.
   void ActualDestroy();
 
   // Configures |media_codec_| with the given codec parameters from the client.
   // This configuration will (probably) not be complete before this call
   // returns.  Multiple calls before completion will be ignored.  |state_|
   // must be NO_ERROR or WAITING_FOR_CODEC.  Note that, once you call this,
   // you should be careful to avoid modifying members of |codec_config_| until
@@ skipping 95 matching lines @@
   // On platforms which support seamless surface changes, this will reinitialize
   // the picture buffer manager with the new surface. This function reads and
   // clears the surface id from |pending_surface_id_|. It will issue a decode
   // error if the surface change fails. Returns false on failure.
   bool UpdateSurface();
 
   // Release |media_codec_| if it's not null, and notify
   // |picture_buffer_manager_|.
   void ReleaseCodec();
 
+  // Returns the surface ID from the incoming bundle, if we have one, or
+  // the current surface bundle if not.

[watk, 2017/02/22 20:38:55]

Could you give me a mental model for why this is the right behavior? I believe
you that it is, but whenever I see surface_id() written I wonder which one it
is, and whether it matters which one it is.

[liberato (no reviews please), 2017/02/23 18:18:46]

after yet more thought after our offline discussion, i think that it's right as
written after all.  i'll update the comment.

TL;DR: only one of |incoming| and |current_config_->surface_bundle| has an
allocated (or is a waiter for) surface.  UpdateSurface ensures it.

+  int surface_id() const {
+    return incoming_bundle_ ? incoming_bundle_->surface_id
+                            : codec_config_->surface_bundle->surface_id;
+  }
+
   // Used to DCHECK that we are called on the correct thread.
   base::ThreadChecker thread_checker_;
 
   // To expose client callbacks from VideoDecodeAccelerator.
   Client* client_;
 
   AVDACodecAllocator* codec_allocator_;
 
   // Callback to set the correct gl context.
   MakeGLContextCurrentCallback make_context_current_cb_;
@@ skipping 93 matching lines @@
 
   // True if surface creation and |picture_buffer_manager_| initialization has
   // been defered until the first Decode() call.
   bool defer_surface_creation_;
 
   // Has a value if a SetSurface() call has occurred and a new surface should be
   // switched to when possible. Cleared during OnSurfaceDestroyed() and if all
   // pictures have been rendered in DequeueOutput().
   base::Optional<int32_t> pending_surface_id_;
 
-  // The task type used for the last codec release. For posting SurfaceTexture
-  // release to the same thread.
-  TaskType last_release_task_type_;
-
   // Copy of the VDA::Config we were given.
   Config config_;
 
+  // SurfaceBundle that we're going to use for StartSurfaceCreation.  This is
+  // separate than the bundle in |codec_config_|, since we can start surface
+  // creation while another codec is using the old surface.  For example, if
+  // we're going to SetSurface, then the current codec will depend on the
+  // current bundle until then.
+  scoped_refptr<AVDASurfaceBundle> incoming_bundle_;
+
   // WeakPtrFactory for posting tasks back to |this|.
   base::WeakPtrFactory<AndroidVideoDecodeAccelerator> weak_this_factory_;
 
   friend class AndroidVideoDecodeAcceleratorTest;
 };
 
 }  // namespace media
 
 #endif  // MEDIA_GPU_ANDROID_VIDEO_DECODE_ACCELERATOR_H_