[Cast] Size-Adaptable platform video encoders.

media/cast/sender/h264_vt_encoder.cc

 // Copyright 2014 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/cast/sender/h264_vt_encoder.h"
 
 #include <string>
 #include <vector>
 
 #include "base/big_endian.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/synchronization/lock.h"
 #include "media/base/mac/corevideo_glue.h"
 #include "media/base/mac/video_frame_mac.h"
 #include "media/cast/sender/video_frame_factory.h"
 
 namespace media {
 namespace cast {
 
 namespace {
 
 // Container for the associated data of a video frame being processed.
@@ skipping 169 matching lines @@
     CopyNalsToAnnexB<uint32_t>(bb_data, bb_size, annexb_buffer);
   } else {
     NOTREACHED();
   }
 }
 
 // Implementation of the VideoFrameFactory interface using |CVPixelBufferPool|.
 class VideoFrameFactoryCVPixelBufferPoolImpl : public VideoFrameFactory {
  public:
   VideoFrameFactoryCVPixelBufferPoolImpl(
-      const base::ScopedCFTypeRef<CVPixelBufferPoolRef>& pool)
-      : pool_(pool) {}
+      const base::ScopedCFTypeRef<CVPixelBufferPoolRef>& pool,
+      const gfx::Size& frame_size)
+      : pool_(pool),
+        frame_size_(frame_size) {}
 
   ~VideoFrameFactoryCVPixelBufferPoolImpl() override {}
 
-  scoped_refptr<VideoFrame> CreateFrame(base::TimeDelta timestamp) override {
+  scoped_refptr<VideoFrame> MaybeCreateFrame(
+      const gfx::Size& frame_size, base::TimeDelta timestamp) override {
+    if (frame_size != frame_size_)
+      return nullptr;  // Buffer pool is not a match for requested frame size.

[jfroy, 2015/02/10 22:51:37]

Not sure about chromium style, but maybe wrap in {}?

[miu, 2015/02/11 00:31:24]

Unfortunately, in Chromium the style is to NOT wrap one-liner then clauses in
braces.  I wish it weren't so...

+
     base::ScopedCFTypeRef<CVPixelBufferRef> buffer;
-    CHECK_EQ(kCVReturnSuccess,
-             CVPixelBufferPoolCreatePixelBuffer(kCFAllocatorDefault, pool_,
-                                                buffer.InitializeInto()));
+    if (CVPixelBufferPoolCreatePixelBuffer(kCFAllocatorDefault, pool_,
+                                           buffer.InitializeInto()) !=
+            kCVReturnSuccess ||
+        !buffer)

[jfroy, 2015/02/10 22:51:37]

There should never be a case where buffer == nullptr and the return value ==
kCVReturnSuccess. If you have seen such a case, then that's bananas. Otherwise,
I'd just check the return value and maybe DCHECK the buffer after.

[miu, 2015/02/11 00:31:24]

Done.

+      return nullptr;  // Buffer pool has run out of pixel buffers.

[jfroy, 2015/02/10 22:51:37]

Not sure about chromium style, but maybe wrap in {}?

[miu, 2015/02/11 00:31:25]

Acknowledged.

+
+    // On some Macs, the buffer pool seems to return pixel buffers without the
+    // required attachments set.  Set them explicitly here, and they must be
+    // in-sync with those in H264VideoToolboxEncoder::ConfigureSession().
+    CVBufferSetAttachment(buffer, kCVImageBufferColorPrimariesKey,

[jfroy, 2015/02/10 22:51:37]

As discussed, instead of setting these attributes here on each buffer, add a
'kCVBufferPropagatedAttachmentsKey' kv pair to the 'buffer_attributes' argument
passed to VTCompressionSessionCreate() in Initialize(). The value should be a
dictionary containing each of the colorimetric kv pairs set here.

[miu, 2015/02/11 00:31:24]

Done.

+                          kCVImageBufferColorPrimaries_ITU_R_709_2,
+                          kCVAttachmentMode_ShouldPropagate);
+    CVBufferSetAttachment(buffer, kCVImageBufferTransferFunctionKey,
+                          kCVImageBufferTransferFunction_ITU_R_709_2,
+                          kCVAttachmentMode_ShouldPropagate);
+    CVBufferSetAttachment(buffer, kCVImageBufferYCbCrMatrixKey,
+                          kCVImageBufferYCbCrMatrix_ITU_R_709_2,
+                          kCVAttachmentMode_ShouldPropagate);
+
     return VideoFrame::WrapCVPixelBuffer(buffer, timestamp);
   }
 
  private:
-  base::ScopedCFTypeRef<CVPixelBufferPoolRef> pool_;
+  const base::ScopedCFTypeRef<CVPixelBufferPoolRef> pool_;
+  const gfx::Size frame_size_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoFrameFactoryCVPixelBufferPoolImpl);
 };
 
 }  // namespace
 
+// static
+bool H264VideoToolboxEncoder::IsSupported(
+    const VideoSenderConfig& video_config) {
+  return video_config.codec == CODEC_VIDEO_H264 && VideoToolboxGlue::Get();
+}
+
 H264VideoToolboxEncoder::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)
     : cast_environment_(cast_environment),
       videotoolbox_glue_(VideoToolboxGlue::Get()),
-      frame_id_(kStartFrameId),
+      frame_size_(frame_size),
+      status_change_cb_(status_change_cb),
+      next_frame_id_(first_frame_id),
       encode_next_frame_as_keyframe_(false) {
-  DCHECK(!frame_size.IsEmpty());
-  DCHECK(!status_change_cb.is_null());
+  DCHECK(!frame_size_.IsEmpty());
+  DCHECK(!status_change_cb_.is_null());
 
   OperationalStatus operational_status;
   if (video_config.codec == CODEC_VIDEO_H264 && videotoolbox_glue_) {
-    operational_status = Initialize(video_config, frame_size) ?
+    operational_status = Initialize(video_config) ?
         STATUS_INITIALIZED : STATUS_INVALID_CONFIGURATION;
   } else {
     operational_status = STATUS_UNSUPPORTED_CODEC;
   }
   cast_environment_->PostTask(
       CastEnvironment::MAIN,
       FROM_HERE,
-      base::Bind(status_change_cb, operational_status));
+      base::Bind(status_change_cb_, operational_status));
 }
 
 H264VideoToolboxEncoder::~H264VideoToolboxEncoder() {
   Teardown();
 }
 
 bool H264VideoToolboxEncoder::Initialize(
-    const VideoSenderConfig& video_config,
-    const gfx::Size& frame_size) {
+    const VideoSenderConfig& video_config) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!compression_session_);
 
   // Note that the encoder object is given to the compression session as the
   // callback context using a raw pointer. The C API does not allow us to use
   // a smart pointer, nor is this encoder ref counted. However, this is still
   // safe, because we 1) we own the compression session and 2) we tear it down
   // safely. When destructing the encoder, the compression session is flushed
   // and invalidated. Internally, VideoToolbox will join all of its threads
   // before returning to the client. Therefore, when control returns to us, we
@@ skipping 14 matching lines @@
   const int formats[] = {
       kCVPixelFormatType_420YpCbCr8Planar,
       CoreVideoGlue::kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange};
   base::ScopedCFTypeRef<CFArrayRef> formats_array = ArrayWithIntegers(
       std::vector<int>(formats, formats + arraysize(formats)));
   base::ScopedCFTypeRef<CFDictionaryRef> buffer_attributes =
       DictionaryWithKeyValue(kCVPixelBufferPixelFormatTypeKey, formats_array);
 
   VTCompressionSessionRef session;
   OSStatus status = videotoolbox_glue_->VTCompressionSessionCreate(
-      kCFAllocatorDefault, frame_size.width(), frame_size.height(),
+      kCFAllocatorDefault, frame_size_.width(), frame_size_.height(),
       CoreMediaGlue::kCMVideoCodecType_H264, encoder_spec, buffer_attributes,
       nullptr /* compressedDataAllocator */,
       &H264VideoToolboxEncoder::CompressionCallback,
       reinterpret_cast<void*>(this), &session);
   if (status != noErr) {
     DLOG(ERROR) << " VTCompressionSessionCreate failed: " << status;
     return false;
   }
   compression_session_.reset(session);
 
@@ skipping 19 matching lines @@
           ->kVTCompressionPropertyKey_MaxKeyFrameIntervalDuration(),
       240);
   // TODO(jfroy): implement better bitrate control
   //              https://crbug.com/425352
   SetSessionProperty(
       videotoolbox_glue_->kVTCompressionPropertyKey_AverageBitRate(),
       (video_config.min_bitrate + video_config.max_bitrate) / 2);
   SetSessionProperty(
       videotoolbox_glue_->kVTCompressionPropertyKey_ExpectedFrameRate(),
       video_config.max_frame_rate);
+  // Keep these attachment settings in-sync with those in
+  // VideoFrameFactoryCVPixelBufferPoolImpl::MaybeCreateFrame().

[jfroy, 2015/02/10 22:51:37]

Update this comment if the colorimetric keys are moved to Initialize(). And yes,
it's kind of a shitty that these keys don't influence the buffers that are
vended by the pool! They are instead used to encode the SPS and PPS NAL units :/

[miu, 2015/02/11 00:31:25]

Done.

   SetSessionProperty(
       videotoolbox_glue_->kVTCompressionPropertyKey_ColorPrimaries(),
       kCVImageBufferColorPrimaries_ITU_R_709_2);
   SetSessionProperty(
       videotoolbox_glue_->kVTCompressionPropertyKey_TransferFunction(),
       kCVImageBufferTransferFunction_ITU_R_709_2);
   SetSessionProperty(
       videotoolbox_glue_->kVTCompressionPropertyKey_YCbCrMatrix(),
       kCVImageBufferYCbCrMatrix_ITU_R_709_2);
   if (video_config.max_number_of_video_buffers_used > 0) {
     SetSessionProperty(
         videotoolbox_glue_->kVTCompressionPropertyKey_MaxFrameDelayCount(),
         video_config.max_number_of_video_buffers_used);
   }
 }
 
 void H264VideoToolboxEncoder::Teardown() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // If the compression session exists, invalidate it. This blocks until all
   // pending output callbacks have returned and any internal threads have
   // joined, ensuring no output callback ever sees a dangling encoder pointer.
   if (compression_session_) {
     videotoolbox_glue_->VTCompressionSessionInvalidate(compression_session_);
     compression_session_.reset();
   }
 }
 
-bool H264VideoToolboxEncoder::CanEncodeVariedFrameSizes() const {
-  return false;
-}
-
 bool H264VideoToolboxEncoder::EncodeVideoFrame(
     const scoped_refptr<media::VideoFrame>& video_frame,
     const base::TimeTicks& reference_time,
     const FrameEncodedCallback& frame_encoded_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(!video_frame->visible_rect().IsEmpty());
   DCHECK(!frame_encoded_callback.is_null());
 
   if (!compression_session_) {
     DLOG(ERROR) << " compression session is null";
     return false;
   }
 
+  if (video_frame->visible_rect().size() != frame_size_)
+    return false;

[jfroy, 2015/02/10 22:51:37]

Not sure about chromium style, but maybe wrap in {}?

[miu, 2015/02/11 00:31:24]

Acknowledged.

+
   // Wrap the VideoFrame in a CVPixelBuffer. In all cases, no data will be
   // copied. If the VideoFrame was created by this encoder's video frame
   // factory, then the returned CVPixelBuffer will have been obtained from the
   // compression session's pixel buffer pool. This will eliminate a copy of the
   // frame into memory visible by the hardware encoder. The VideoFrame's
   // lifetime is extended for the lifetime of the returned CVPixelBuffer.
   auto pixel_buffer = media::WrapVideoFrameInCVPixelBuffer(*video_frame);
   if (!pixel_buffer) {
     return false;
   }
@@ skipping 41 matching lines @@
 
   encode_next_frame_as_keyframe_ = true;
 }
 
 void H264VideoToolboxEncoder::LatestFrameIdToReference(uint32 /*frame_id*/) {
   // Not supported by VideoToolbox in any meaningful manner.
 }
 
 scoped_ptr<VideoFrameFactory>
 H264VideoToolboxEncoder::CreateVideoFrameFactory() {
+  if (!videotoolbox_glue_ || !compression_session_)
+    return nullptr;

[jfroy, 2015/02/10 22:51:37]

Not sure about chromium style, but maybe wrap in {}?

[miu, 2015/02/11 00:31:25]

Acknowledged.

   base::ScopedCFTypeRef<CVPixelBufferPoolRef> pool(
       videotoolbox_glue_->VTCompressionSessionGetPixelBufferPool(
           compression_session_),
       base::scoped_policy::RETAIN);
   return scoped_ptr<VideoFrameFactory>(
-      new VideoFrameFactoryCVPixelBufferPoolImpl(pool));
+      new VideoFrameFactoryCVPixelBufferPoolImpl(pool, frame_size_));
 }
 
 void H264VideoToolboxEncoder::EmitFrames() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!compression_session_) {
     DLOG(ERROR) << " compression session is null";
     return;
   }
 
@@ skipping 22 matching lines @@
                                                  CFStringRef value) {
   return videotoolbox_glue_->VTSessionSetProperty(compression_session_, key,
                                                   value) == noErr;
 }
 
 void H264VideoToolboxEncoder::CompressionCallback(void* encoder_opaque,
                                                   void* request_opaque,
                                                   OSStatus status,
                                                   VTEncodeInfoFlags info,
                                                   CMSampleBufferRef sbuf) {
-  if (status != noErr) {
-    DLOG(ERROR) << " encoding failed: " << status;
-    return;
-  }
-  if ((info & VideoToolboxGlue::kVTEncodeInfo_FrameDropped)) {
-    DVLOG(2) << " frame dropped";
-    return;
-  }
-
   auto encoder = reinterpret_cast<H264VideoToolboxEncoder*>(encoder_opaque);
   const scoped_ptr<InProgressFrameEncode> request(
       reinterpret_cast<InProgressFrameEncode*>(request_opaque));
-  auto sample_attachments = static_cast<CFDictionaryRef>(CFArrayGetValueAtIndex(
-      CoreMediaGlue::CMSampleBufferGetSampleAttachmentsArray(sbuf, true), 0));
+  bool keyframe = false;
+  bool has_frame_data = false;
 
-  // If the NotSync key is not present, it implies Sync, which indicates a
-  // keyframe (at least I think, VT documentation is, erm, sparse). Could
-  // alternatively use kCMSampleAttachmentKey_DependsOnOthers == false.
-  bool keyframe =
-      !CFDictionaryContainsKey(sample_attachments,
-                               CoreMediaGlue::kCMSampleAttachmentKey_NotSync());
+  if (status != noErr) {
+    DLOG(ERROR) << " encoding failed: " << status;
+    encoder->cast_environment_->PostTask(
+        CastEnvironment::MAIN,
+        FROM_HERE,
+        base::Bind(encoder->status_change_cb_, STATUS_CODEC_RUNTIME_ERROR));
+  } else if ((info & VideoToolboxGlue::kVTEncodeInfo_FrameDropped)) {
+    DVLOG(2) << " frame dropped";
+  } else {
+    auto sample_attachments = static_cast<CFDictionaryRef>(
+        CFArrayGetValueAtIndex(
+            CoreMediaGlue::CMSampleBufferGetSampleAttachmentsArray(sbuf, true),
+            0));
+
+    // If the NotSync key is not present, it implies Sync, which indicates a
+    // keyframe (at least I think, VT documentation is, erm, sparse). Could
+    // alternatively use kCMSampleAttachmentKey_DependsOnOthers == false.
+    keyframe = !CFDictionaryContainsKey(
+        sample_attachments,
+        CoreMediaGlue::kCMSampleAttachmentKey_NotSync());
+    has_frame_data = true;
+  }
 
   // Increment the encoder-scoped frame id and assign the new value to this
   // frame. VideoToolbox calls the output callback serially, so this is safe.
-  uint32 frame_id = ++encoder->frame_id_;
+  const uint32 frame_id = encoder->next_frame_id_++;
 
   scoped_ptr<EncodedFrame> encoded_frame(new EncodedFrame());
   encoded_frame->frame_id = frame_id;
   encoded_frame->reference_time = request->reference_time;
   encoded_frame->rtp_timestamp = request->rtp_timestamp;
   if (keyframe) {
     encoded_frame->dependency = EncodedFrame::KEY;
     encoded_frame->referenced_frame_id = frame_id;
   } else {
     encoded_frame->dependency = EncodedFrame::DEPENDENT;
     // H.264 supports complex frame reference schemes (multiple reference
     // frames, slice references, backward and forward references, etc). Cast
     // doesn't support the concept of forward-referencing frame dependencies or
     // multiple frame dependencies; so pretend that all frames are only
     // decodable after their immediately preceding frame is decoded. This will
     // ensure a Cast receiver only attempts to decode the frames sequentially
     // and in order. Furthermore, the encoder is configured to never use forward
     // references (see |kVTCompressionPropertyKey_AllowFrameReordering|). There
     // is no way to prevent multiple reference frames.
     encoded_frame->referenced_frame_id = frame_id - 1;
   }
 
-  CopySampleBufferToAnnexBBuffer(sbuf, &encoded_frame->data, keyframe);
+  if (has_frame_data)
+    CopySampleBufferToAnnexBBuffer(sbuf, &encoded_frame->data, keyframe);

[jfroy, 2015/02/10 22:51:37]

Not sure about chromium style, but maybe wrap in {}?

[miu, 2015/02/11 00:31:25]

Acknowledged.

 
   encoder->cast_environment_->PostTask(
       CastEnvironment::MAIN, FROM_HERE,
       base::Bind(request->frame_encoded_callback,
                  base::Passed(&encoded_frame)));
 }
 
+// A ref-counted structure that is shared to provide concurrent access to the
+// VideoFrameFactory instance for the current encoder.  OnEncoderReplaced() can
+// change |factory| whenever an encoder instance has been replaced, while users
+// of CreateVideoFrameFactory() may attempt to read/use |factory| by any thread
+// at any time.
+struct SizeAdaptableH264VideoToolboxVideoEncoder::FactoryHolder
+    : public base::RefCountedThreadSafe<FactoryHolder> {
+  base::Lock lock;
+  scoped_ptr<VideoFrameFactory> factory;
+
+ private:
+  friend class base::RefCountedThreadSafe<FactoryHolder>;
+  ~FactoryHolder() {}
+};
+
+SizeAdaptableH264VideoToolboxVideoEncoder::
+    SizeAdaptableH264VideoToolboxVideoEncoder(
+        const scoped_refptr<CastEnvironment>& cast_environment,
+        const VideoSenderConfig& video_config,
+        const StatusChangeCallback& status_change_cb)
+        : SizeAdaptableVideoEncoderBase(cast_environment,
+                                        video_config,
+                                        status_change_cb),
+          holder_(new FactoryHolder()) {}
+
+SizeAdaptableH264VideoToolboxVideoEncoder::
+    ~SizeAdaptableH264VideoToolboxVideoEncoder() {}
+
+scoped_ptr<VideoFrameFactory>
+    SizeAdaptableH264VideoToolboxVideoEncoder::CreateVideoFrameFactory() {
+  // A proxy allowing SizeAdaptableH264VideoToolboxVideoEncoder to swap out the
+  // VideoFrameFactory instance to match one appropriate for the current encoder
+  // instance.
+  class VideoFrameFactoryProxy : public VideoFrameFactory {

[hubbe, 2015/02/11 00:47:54]

Personally I think local classes are harder to read.
Can we just put this in an anonymous namespace before the function instead?

[miu, 2015/02/11 02:14:23]

Done.

+   public:
+    explicit VideoFrameFactoryProxy(const scoped_refptr<FactoryHolder>& holder)
+        : holder_(holder) {}
+
+    ~VideoFrameFactoryProxy() override {}
+
+    scoped_refptr<VideoFrame> MaybeCreateFrame(
+        const gfx::Size& frame_size, base::TimeDelta timestamp) override {
+      base::AutoLock auto_lock(holder_->lock);
+      return holder_->factory ?
+          holder_->factory->MaybeCreateFrame(frame_size, timestamp) : nullptr;
+    }
+
+   private:
+    const scoped_refptr<FactoryHolder> holder_;
+
+    DISALLOW_COPY_AND_ASSIGN(VideoFrameFactoryProxy);
+  };
+
+  return scoped_ptr<VideoFrameFactory>(new VideoFrameFactoryProxy(holder_));
+}
+
+scoped_ptr<VideoEncoder>
+    SizeAdaptableH264VideoToolboxVideoEncoder::CreateReplacementEncoder() {
+  return scoped_ptr<VideoEncoder>(new H264VideoToolboxEncoder(
+      cast_environment(),
+      video_config(),
+      next_encoder_frame_size(),

[hubbe, 2015/02/11 00:47:54]

It feels a little awkward to use "next_encoder_frame_size()" here.
Maybe size should be an argument?

[miu, 2015/02/11 02:14:23]

Eliminated "next_encoder" part of things, as discussed.

+      last_frame_id() + 1,
+      CreateEncoderStatusChangeCallback()));
+}
+
+void SizeAdaptableH264VideoToolboxVideoEncoder::OnEncoderReplaced(
+    VideoEncoder* replacement_encoder) {
+  scoped_ptr<VideoFrameFactory> current_factory(
+      replacement_encoder->CreateVideoFrameFactory());
+  base::AutoLock auto_lock(holder_->lock);
+  holder_->factory = current_factory.Pass();

[hubbe, 2015/02/11 00:47:54]

Call baseclass OnEncoderReplaced()

[miu, 2015/02/11 02:14:23]

Done.

+}
+
+void SizeAdaptableH264VideoToolboxVideoEncoder::DestroyCurrentEncoder() {
+  {
+    base::AutoLock auto_lock(holder_->lock);
+    holder_->factory.reset();
+  }
+  SizeAdaptableVideoEncoderBase::DestroyCurrentEncoder();
+}
+
 }  // namespace cast
 }  // namespace media