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

webrtc/common_video/i420_buffer_pool.cc

Index: webrtc/common_video/i420_buffer_pool.cc
diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc
index 82a10797b353d3b9978411f1ce9e1c7edf878748..bb2ac2ffc26df05148d1c3f8891d97ef8c9f3494 100644
--- a/webrtc/common_video/i420_buffer_pool.cc
+++ b/webrtc/common_video/i420_buffer_pool.cc
@@ -15,6 +15,9 @@
 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(
@@ -26,18 +29,27 @@ class PooledI420Buffer : public webrtc::VideoFrameBuffer {
 
   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 {