[cast] Handle frame size changes directly in the VideoToolbox encoder.

To implement various kinds of re-initialization conditions more easily,
this CL subsumes responsibility for handling frame size changes directly
in the VideoToolbox encoder.

The design is very similar to the previous one with a proxy encoder.
Instead of proxying the entire encoder, only the video frame factory is
proxied. Both the encoder and the proxy own a ref-counted reference to
the factory, which in turn owns a weak back-reference to the encoder and
a ref-counted reference to the current pixel buffer pool.

When a frame size change is detected either by the encoder or by the
video frame factory, the internal compression session is reset. This is
done synchronously when executing on the Cast main thread, and by
posting a task to the cast main thread when not.

The code to re-initialize the compression session will be re-used in
upcoming work where additional conditions, such as backgrounding, need
to be monitored for session reinitialization.

This new CL fixes the cast unit tests by modifying the tests to handle
encoders that do not have a frame delay when changing the video size.

R=dcaiafa@chromium.org, miu@chromium.org
BUG=477895

Review URL: https://codereview.chromium.org/1100643002

Cr-Commit-Position: refs/heads/master@{#326907}

media/cast/sender/h264_vt_encoder.h

@@ -16,7 +16,8 @@ namespace cast {
 
 // VideoToolbox implementation of the media::cast::VideoEncoder interface.
 // VideoToolbox makes no guarantees that it is thread safe, so this object is
-// pinned to the thread on which it is constructed.
+// pinned to the thread on which it is constructed. Supports changing frame
+// sizes directly.
 class H264VideoToolboxEncoder : public VideoEncoder {
   typedef CoreMediaGlue::CMSampleBufferRef CMSampleBufferRef;
   typedef VideoToolboxGlue::VTCompressionSessionRef VTCompressionSessionRef;
@@ -30,8 +31,6 @@ class H264VideoToolboxEncoder : public VideoEncoder {
   H264VideoToolboxEncoder(
       const scoped_refptr<CastEnvironment>& cast_environment,
       const VideoSenderConfig& video_config,
-      const gfx::Size& frame_size,
-      uint32 first_frame_id,
       const StatusChangeCallback& status_change_cb);
   ~H264VideoToolboxEncoder() override;
 
@@ -47,14 +46,25 @@ class H264VideoToolboxEncoder : public VideoEncoder {
   void EmitFrames() override;
 
  private:
-  // Initialize the compression session.
-  bool Initialize(const VideoSenderConfig& video_config);
+  // VideoFrameFactory tied to the VideoToolbox encoder.
+  class VideoFrameFactoryImpl;
 
-  // Configure the compression session.
-  void ConfigureSession(const VideoSenderConfig& video_config);
+  // Reset the encoder's compression session by destroying the existing one
+  // using DestroyCompressionSession() and creating a new one. The new session
+  // is configured using ConfigureCompressionSession().
+  void ResetCompressionSession();
 
-  // Teardown the encoder.
-  void Teardown();
+  // Configure the current compression session using current encoder settings.
+  void ConfigureCompressionSession();
+
+  // Destroy the current compression session if any. Blocks until all pending
+  // frames have been flushed out (similar to EmitFrames without doing any
+  // encoding work).
+  void DestroyCompressionSession();
+
+  // Update the encoder's target frame size by resetting the compression
+  // session. This will also update the video frame factory.
+  void UpdateFrameSize(const gfx::Size& size_needed);
 
   // Set a compression session property.
   bool SetSessionProperty(CFStringRef key, int32_t value);
@@ -74,8 +84,14 @@ class H264VideoToolboxEncoder : public VideoEncoder {
   // VideoToolboxGlue provides access to VideoToolbox at runtime.
   const VideoToolboxGlue* const videotoolbox_glue_;
 
-  // The size of the visible region of the video frames to be encoded.
-  const gfx::Size frame_size_;
+  // VideoSenderConfig copy so we can create compression sessions on demand.
+  // This is needed to recover from backgrounding and other events that can
+  // invalidate compression sessions.
+  const VideoSenderConfig video_config_;
+
+  // Frame size of the current compression session. Can be changed by submitting
+  // a frame of a different size, which will cause a compression session reset.
+  gfx::Size frame_size_;
 
   // Callback used to report initialization status and runtime errors.
   const StatusChangeCallback status_change_cb_;
@@ -86,41 +102,19 @@ class H264VideoToolboxEncoder : public VideoEncoder {
   // The compression session.
   base::ScopedCFTypeRef<VTCompressionSessionRef> compression_session_;
 
-  // Frame identifier counter.
-  uint32 next_frame_id_;
+  // Video frame factory tied to the encoder.
+  scoped_refptr<VideoFrameFactoryImpl> video_frame_factory_;
+
+  // The ID of the last frame that was emitted.
+  uint32 last_frame_id_;
 
   // Force next frame to be a keyframe.
   bool encode_next_frame_as_keyframe_;
 
-  DISALLOW_COPY_AND_ASSIGN(H264VideoToolboxEncoder);
-};
+  // NOTE: Weak pointers must be invalidated before all other member variables.
+  base::WeakPtrFactory<H264VideoToolboxEncoder> weak_factory_;
 
-// An implementation of SizeAdaptableVideoEncoderBase to proxy for
-// H264VideoToolboxEncoder instances.
-class SizeAdaptableH264VideoToolboxVideoEncoder
- : public SizeAdaptableVideoEncoderBase {
- public:
-  SizeAdaptableH264VideoToolboxVideoEncoder(
-      const scoped_refptr<CastEnvironment>& cast_environment,
-      const VideoSenderConfig& video_config,
-      const StatusChangeCallback& status_change_cb);
-
-  ~SizeAdaptableH264VideoToolboxVideoEncoder() override;
-
-  scoped_ptr<VideoFrameFactory> CreateVideoFrameFactory() override;
-
- protected:
-  scoped_ptr<VideoEncoder> CreateEncoder() override;
-  void OnEncoderReplaced(VideoEncoder* replacement_encoder) override;
-  void DestroyEncoder() override;
-
- private:
-  struct FactoryHolder;
-  class VideoFrameFactoryProxy;
-
-  const scoped_refptr<FactoryHolder> holder_;
-
-  DISALLOW_COPY_AND_ASSIGN(SizeAdaptableH264VideoToolboxVideoEncoder);
+  DISALLOW_COPY_AND_ASSIGN(H264VideoToolboxEncoder);
 };
 
 }  // namespace cast

media/cast/sender/h264_vt_encoder_unittest.cc

@@ -208,8 +208,6 @@ class H264VideoToolboxEncoderTest : public ::testing::Test {
     encoder_.reset(new H264VideoToolboxEncoder(
         cast_environment_,
         video_sender_config_,
-        gfx::Size(kVideoWidth, kVideoHeight),
-        0u,
         base::Bind(&SaveOperationalStatus, &operational_status_)));
     message_loop_.RunUntilIdle();
     EXPECT_EQ(STATUS_INITIALIZED, operational_status_);
@@ -306,10 +304,12 @@ TEST_F(H264VideoToolboxEncoderTest, CheckFramesAreDecodable) {
 TEST_F(H264VideoToolboxEncoderTest, CheckVideoFrameFactory) {
   auto video_frame_factory = encoder_->CreateVideoFrameFactory();
   ASSERT_TRUE(video_frame_factory.get());
-  CreateFrameAndMemsetPlane(video_frame_factory.get());
-  // TODO(jfroy): Need to test that the encoder can encode VideoFrames provided
-  // by the VideoFrameFactory.
-  encoder_.reset();
+  // The first call to |MaybeCreateFrame| will return null but post a task to
+  // the encoder to initialize for the specified frame size. We then drain the
+  // message loop. After that, the encoder should have initialized and we
+  // request a frame again.
+  ASSERT_FALSE(video_frame_factory->MaybeCreateFrame(
+      gfx::Size(kVideoWidth, kVideoHeight), base::TimeDelta()));
   message_loop_.RunUntilIdle();
   CreateFrameAndMemsetPlane(video_frame_factory.get());
 }

media/cast/sender/video_encoder.cc

@@ -26,11 +26,8 @@ scoped_ptr<VideoEncoder> VideoEncoder::Create(
 #if defined(OS_MACOSX) || defined(OS_IOS)
   if (!video_config.use_external_encoder &&
       H264VideoToolboxEncoder::IsSupported(video_config)) {
-    return scoped_ptr<VideoEncoder>(
-        new SizeAdaptableH264VideoToolboxVideoEncoder(
-            cast_environment,
-            video_config,
-            status_change_cb));
+    return scoped_ptr<VideoEncoder>(new H264VideoToolboxEncoder(
+        cast_environment, video_config, status_change_cb));
   }
 #endif  // defined(OS_MACOSX)
 

media/cast/sender/video_encoder_unittest.cc

@@ -101,6 +101,10 @@ class VideoEncoderTest
         is_testing_video_toolbox_encoder();
   }
 
+  bool encoder_has_resize_delay() const {
+    return is_testing_platform_encoder() && !is_testing_video_toolbox_encoder();
+  }
+
   VideoEncoder* video_encoder() const {
     return video_encoder_.get();
   }
@@ -282,14 +286,13 @@ TEST_P(VideoEncoderTest, GeneratesKeyFrameThenOnlyDeltaFrames) {
   uint32 reference_frame_id = 0;
   const gfx::Size frame_size(1280, 720);
 
-  // For the platform encoders, the first one or more frames is dropped while
-  // the encoder initializes.  Then, for all encoders, expect one key frame is
-  // delivered.
+  // Some encoders drop one or more frames initially while the encoder
+  // initializes. Then, for all encoders, expect one key frame is delivered.
   bool accepted_first_frame = false;
   do {
     accepted_first_frame = EncodeAndCheckDelivery(
         CreateTestVideoFrame(frame_size), frame_id, reference_frame_id);
-    if (!is_testing_platform_encoder())
+    if (!encoder_has_resize_delay())
       EXPECT_TRUE(accepted_first_frame);
     RunTasksAndAdvanceClock();
   } while (!accepted_first_frame);
@@ -365,28 +368,28 @@ TEST_P(VideoEncoderTest, EncodesVariedFrameSizes) {
 
   uint32 frame_id = 0;
 
-  // Encode one frame at each size.  For the platform encoders, expect no frames
-  // to be delivered since each frame size change will sprun re-initialization
-  // of the underlying encoder.  Otherwise, expect all key frames to come out.
+  // Encode one frame at each size. For encoders with a resize delay, except no
+  // frames to be delivered since each frame size change will sprun
+  // re-initialization of the underlying encoder. Otherwise expect all key
+  // frames to come out.
   for (const auto& frame_size : frame_sizes) {
-    EXPECT_EQ(!is_testing_platform_encoder(),
-              EncodeAndCheckDelivery(CreateTestVideoFrame(frame_size),
-                                     frame_id,
+    EXPECT_EQ(!encoder_has_resize_delay(),
+              EncodeAndCheckDelivery(CreateTestVideoFrame(frame_size), frame_id,
                                      frame_id));
     RunTasksAndAdvanceClock();
-    if (!is_testing_platform_encoder())
+    if (!encoder_has_resize_delay())
       ++frame_id;
   }
 
-  // Encode 10+ frames at each size.  For the platform decoders, expect the
-  // first one or more frames are dropped while the encoder re-inits.  Then, for
-  // all encoders, expect one key frame followed by all delta frames.
+  // Encode 10+ frames at each size. For encoders with a resize delay, expect
+  // the first one or more frames are dropped while the encoder re-inits. Then,
+  // for all encoders, expect one key frame followed by all delta frames.
   for (const auto& frame_size : frame_sizes) {
     bool accepted_first_frame = false;
     do {
       accepted_first_frame = EncodeAndCheckDelivery(
           CreateTestVideoFrame(frame_size), frame_id, frame_id);
-      if (!is_testing_platform_encoder())
+      if (!encoder_has_resize_delay())
         EXPECT_TRUE(accepted_first_frame);
       RunTasksAndAdvanceClock();
     } while (!accepted_first_frame);

media/cast/sender/video_frame_factory.h

@@ -27,9 +27,7 @@ namespace cast {
 // Clients are responsible for serialzing access to a |VideoFrameFactory|.
 // Generally speaking, it is expected that a client will be using these objects
 // from a rendering thread or callback (which may execute on different threads
-// but never concurrently with itself). Forcing every implementation to take a
-// lock, even with no contention, is an unnecessary cost, especially on mobile
-// platforms.
+// but never concurrently with itself).
 class VideoFrameFactory {
  public:
   virtual ~VideoFrameFactory() {}
@@ -39,7 +37,10 @@ class VideoFrameFactory {
   // with the encoder. The format is guaranteed to be I420 or NV12.
   //
   // This can transiently return null if the encoder is not yet initialized or
-  // is re-initializing.
+  // is re-initializing. Note however that if an encoder does support optimized
+  // frames, its |VideoFrameFactory| must eventually return frames. In practice,
+  // this means that |MaybeCreateFrame| must somehow signal the encoder to
+  // perform whatever initialization is needed to eventually produce frames.
   virtual scoped_refptr<VideoFrame> MaybeCreateFrame(
       const gfx::Size& frame_size, base::TimeDelta timestamp) = 0;
 };