Added new VideoFrameBuffer methods Data[YUV]() etc.

webrtc/common_video/i420_buffer_pool.cc

 /*
  *  Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/common_video/include/i420_buffer_pool.h"
 
 #include "webrtc/base/checks.h"
 
 namespace {
 
 // One extra indirection is needed to make |HasOneRef| work.
+// TODO(nisse): Why not simply inherit I420Buffer, and override

[magjed_webrtc, 2016/04/13 08:30:50]

First, you can't override HasOneRef here, because it will just be overridden
again by RefCountedObject that is added last. Second, how would it help? If you
don't have the extra indirection, you can't distinguish the reference from
I420BufferPool, so HasOneRef would always be false for PooledI420Buffers when
they are in use by clients, which is not what we want.

[nisse-webrtc, 2016/04/13 08:47:08]

I'll investigate this in a separate cl. 

The idea is to demote HasOneRef to protected. Then add a public IsMutable, which
will return always false for some classes (NativeHandleBuffer,
WrapperI420Buffer). For a plain I420Buffer, it will return HasOneRef(). For
PooledI420Buffer, it could return GetRefCount() == 2 (needs a new method in the
refcount machinery).

Corner case is when a pool is destroyed. Then it no longer holds a reference,
and therefore I420PooledBuffer::IsMutable should return false, not true, if it
finds two references. Could have a flag is_pooled, which is cleared by
I420BufferPool::Release.

[perkj_webrtc, 2016/04/14 06:30:42]

please remove this todo then and just keep that thought in a list somewhere
else.

+// HasOneRef? Possibly after renaming to IsExclusive or IsMutable or

[perkj_webrtc, 2016/04/12 07:18:40]

If you want.

+// some such.
 class PooledI420Buffer : public webrtc::VideoFrameBuffer {
  public:
   explicit PooledI420Buffer(
       const rtc::scoped_refptr<webrtc::I420Buffer>& buffer)
       : buffer_(buffer) {}
 
  private:
   ~PooledI420Buffer() override {}
 
   int width() const override { return buffer_->width(); }
   int height() const override { return buffer_->height(); }
-  const uint8_t* data(webrtc::PlaneType type) const override {
-    return buffer_->data(type);
+  const uint8_t* DataY() const override { return buffer_->DataY(); }

[perkj_webrtc, 2016/04/12 07:18:40]

Skip adding getters for Data on cricket::VideoFrame if we want to use
frame_buffer()->DataY instead.

[nisse-webrtc, 2016/04/13 13:29:56]

I don't do that... this is a subclass of VideoFrame*Buffer*.

+  const uint8_t* DataU() const override { return buffer_->DataU(); }
+  const uint8_t* DataV() const override { return buffer_->DataV(); }
+
+  // Make the HasOneRef() check here instead of in |buffer_|, because the pool
+  // also has a reference to |buffer_|.
+  uint8_t* MutableDataY() override {
+    RTC_DCHECK(HasOneRef());
+    return const_cast<uint8_t*>(buffer_->DataY());
   }
-  uint8_t* MutableData(webrtc::PlaneType type) override {
-    // Make the HasOneRef() check here instead of in |buffer_|, because the pool
-    // also has a reference to |buffer_|.
+  uint8_t* MutableDataU() override {
     RTC_DCHECK(HasOneRef());
-    return const_cast<uint8_t*>(buffer_->data(type));
+    return const_cast<uint8_t*>(buffer_->DataU());
   }
-  int stride(webrtc::PlaneType type) const override {
-    return buffer_->stride(type);
+  uint8_t* MutableDataV() override {
+    RTC_DCHECK(HasOneRef());
+    return const_cast<uint8_t*>(buffer_->DataV());
   }
+  int StrideY() const override { return buffer_->StrideY(); }
+  int StrideU() const override { return buffer_->StrideU(); }
+  int StrideV() const override { return buffer_->StrideV(); }
   void* native_handle() const override { return nullptr; }
 
   rtc::scoped_refptr<VideoFrameBuffer> NativeToI420Buffer() override {
     RTC_NOTREACHED();
     return nullptr;
   }
 
   friend class rtc::RefCountedObject<PooledI420Buffer>;
   rtc::scoped_refptr<webrtc::I420Buffer> buffer_;
 };
@@ skipping 35 matching lines @@
   // Allocate new buffer.
   rtc::scoped_refptr<I420Buffer> buffer = new rtc::RefCountedObject<I420Buffer>(
       width, height);
   if (zero_initialize_)
     buffer->InitializeData();
   buffers_.push_back(buffer);
   return new rtc::RefCountedObject<PooledI420Buffer>(buffers_.back());
 }
 
 }  // namespace webrtc