WebRtcVideoFrame: DCHECK exclusive ownership for non-const pixel access

Add some const safety by DCHECK(HasOneRef()) in non-const GetYPlane. This CL also replaces all incorrect non-const calls with const calls for pixel data access in cricket::VideoFrame. It's easy to call the non-const version of e.g. GetYPlane by mistake, even if only const-access is needed. For example:
const scoped_ptr<cricket::VideoFrame> foo;
const uint8_t* y = foo->GetYPlane();
will actually call the non-const version of GetYPlane.

R=mflodman@webrtc.org, perkj@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/39079004

Cr-Commit-Position: refs/heads/master@{#8507}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8507 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/app/webrtc/objc/RTCI420Frame.mm

@@ -55,15 +55,18 @@
 }
 
 - (const uint8_t*)yPlane {
-  return _videoFrame->GetYPlane();
+  const cricket::VideoFrame* const_frame = _videoFrame.get();
+  return const_frame->GetYPlane();
 }
 
 - (const uint8_t*)uPlane {
-  return _videoFrame->GetUPlane();
+  const cricket::VideoFrame* const_frame = _videoFrame.get();
+  return const_frame->GetUPlane();
 }
 
 - (const uint8_t*)vPlane {
-  return _videoFrame->GetVPlane();
+  const cricket::VideoFrame* const_frame = _videoFrame.get();
+  return const_frame->GetVPlane();
 }
 
 - (NSInteger)yPitch {

talk/media/base/videoadapter_unittest.cc

@@ -419,7 +419,8 @@ TEST_F(VideoAdapterTest, BlackOutput) {
   EXPECT_TRUE_WAIT(!capturer_->IsRunning() ||
                    listener_->GetStats().captured_frames >= 10, kWaitTimeout);
   // Verify that the output frame is not black.
-  rtc::scoped_ptr<VideoFrame> adapted_frame(listener_->CopyAdaptedFrame());
+  rtc::scoped_ptr<const VideoFrame> adapted_frame(
+      listener_->CopyAdaptedFrame());
   EXPECT_NE(16, *adapted_frame->GetYPlane());
   EXPECT_NE(128, *adapted_frame->GetUPlane());
   EXPECT_NE(128, *adapted_frame->GetVPlane());

talk/media/base/videoframe.cc

@@ -41,7 +41,7 @@ namespace cricket {
 #define ROUNDTO2(v) (v & ~1)
 
 rtc::StreamResult VideoFrame::Write(rtc::StreamInterface* stream,
-                                          int* error) {
+                                          int* error) const {
   rtc::StreamResult result = rtc::SR_SUCCESS;
   const uint8* src_y = GetYPlane();
   const uint8* src_u = GetUPlane();

talk/media/base/videoframe.h

@@ -159,8 +159,8 @@ class VideoFrame {
   // See webrtc/base/stream.h for a description of StreamResult and error.
   // Error may be NULL. If a non-success value is returned from
   // StreamInterface::Write(), we immediately return with that value.
-  virtual rtc::StreamResult Write(rtc::StreamInterface *stream,
-                                        int *error);
+  virtual rtc::StreamResult Write(rtc::StreamInterface* stream,
+                                  int* error) const;
 
   // Converts the I420 data to RGB of a certain type such as ARGB and ABGR.
   // Returns the frame's actual size, regardless of whether it was written or

talk/media/base/videoframe_unittest.h

@@ -2007,13 +2007,14 @@ class VideoFrameTest : public testing::Test {
 
   void CopyIsRef() {
     rtc::scoped_ptr<T> source(new T);
-    rtc::scoped_ptr<cricket::VideoFrame> target;
+    rtc::scoped_ptr<const cricket::VideoFrame> target;
     ASSERT_TRUE(LoadFrameNoRepeat(source.get()));
     target.reset(source->Copy());
     EXPECT_TRUE(IsEqual(*source, *target, 0));
-    EXPECT_EQ(source->GetYPlane(), target->GetYPlane());
-    EXPECT_EQ(source->GetUPlane(), target->GetUPlane());
-    EXPECT_EQ(source->GetVPlane(), target->GetVPlane());
+    const T* const_source = source.get();
+    EXPECT_EQ(const_source->GetYPlane(), target->GetYPlane());
+    EXPECT_EQ(const_source->GetUPlane(), target->GetUPlane());
+    EXPECT_EQ(const_source->GetVPlane(), target->GetVPlane());
   }
 
   void MakeExclusive() {

talk/media/webrtc/webrtcvideoframe.cc

@@ -207,11 +207,13 @@ const uint8* WebRtcVideoFrame::GetVPlane() const {
 }
 
 uint8* WebRtcVideoFrame::GetYPlane() {
+  DCHECK(video_buffer_->HasOneRef());
   uint8_t* buffer = frame()->Buffer();
   return buffer;
 }
 
 uint8* WebRtcVideoFrame::GetUPlane() {
+  DCHECK(video_buffer_->HasOneRef());
   uint8_t* buffer = frame()->Buffer();
   if (buffer) {
     buffer += (frame()->Width() * frame()->Height());
@@ -220,6 +222,7 @@ uint8* WebRtcVideoFrame::GetUPlane() {
 }
 
 uint8* WebRtcVideoFrame::GetVPlane() {
+  DCHECK(video_buffer_->HasOneRef());
   uint8_t* buffer = frame()->Buffer();
   if (buffer) {
     int uv_size = static_cast<int>(GetChromaSize());
@@ -321,13 +324,17 @@ bool WebRtcVideoFrame::Reset(uint32 format,
     new_height = dw;
   }
 
-  size_t desired_size = SizeOf(new_width, new_height);
-  rtc::scoped_refptr<RefCountedBuffer> video_buffer(
-      new RefCountedBuffer(desired_size));
-  // Since the libyuv::ConvertToI420 will handle the rotation, so the
-  // new frame's rotation should always be 0.
-  Attach(video_buffer.get(), desired_size, new_width, new_height, pixel_width,
-         pixel_height, elapsed_time, time_stamp, webrtc::kVideoRotation_0);
+  // Release reference in |video_buffer| at the end of this scope, so that
+  // |video_buffer_| becomes the sole owner.
+  {
+    size_t desired_size = SizeOf(new_width, new_height);
+    rtc::scoped_refptr<RefCountedBuffer> video_buffer(
+        new RefCountedBuffer(desired_size));
+    // Since the libyuv::ConvertToI420 will handle the rotation, so the
+    // new frame's rotation should always be 0.
+    Attach(video_buffer.get(), desired_size, new_width, new_height, pixel_width,
+           pixel_height, elapsed_time, time_stamp, webrtc::kVideoRotation_0);
+  }
 
   int horiz_crop = ((w - dw) / 2) & ~1;
   // ARGB on Windows has negative height.

webrtc/modules/video_capture/test/video_capture_unittest.cc

@@ -514,9 +514,10 @@ TEST_F(VideoCaptureExternalTest, DISABLED_TestExternalCaptureI420) {
   int uv_width = kTestWidth / 2;
   int y_rows = kTestHeight;
   int uv_rows = kTestHeight / 2;
-  unsigned char* y_plane = test_frame_.buffer(webrtc::kYPlane);
-  unsigned char* u_plane = test_frame_.buffer(webrtc::kUPlane);
-  unsigned char* v_plane = test_frame_.buffer(webrtc::kVPlane);
+  const webrtc::I420VideoFrame& const_test_frame = test_frame_;
+  const unsigned char* y_plane = const_test_frame.buffer(webrtc::kYPlane);
+  const unsigned char* u_plane = const_test_frame.buffer(webrtc::kUPlane);
+  const unsigned char* v_plane = const_test_frame.buffer(webrtc::kVPlane);
   // Copy Y
   unsigned char* current_pointer = aligned_test_frame.buffer(webrtc::kYPlane);
   for (int i = 0; i < y_rows; ++i) {

webrtc/video_engine/vie_capturer_unittest.cc

@@ -94,7 +94,7 @@ class ViECapturerTest : public ::testing::Test {
     data_callback_->OnIncomingCapturedFrame(0, *frame);
   }
 
-  void AddOutputFrame(I420VideoFrame* frame) {
+  void AddOutputFrame(const I420VideoFrame* frame) {
     if (frame->native_handle() == NULL)
       output_frame_ybuffers_.push_back(frame->buffer(kYPlane));
     // Clone the frames because ViECapturer owns the frames.
@@ -126,7 +126,7 @@ class ViECapturerTest : public ::testing::Test {
 
   // The pointers of Y plane buffers of output frames. This is used to verify
   // the frame are swapped and not copied.
-  std::vector<uint8_t*> output_frame_ybuffers_;
+  std::vector<const uint8_t*> output_frame_ybuffers_;
 };
 
 TEST_F(ViECapturerTest, TestTextureFrames) {
@@ -145,10 +145,11 @@ TEST_F(ViECapturerTest, TestTextureFrames) {
 TEST_F(ViECapturerTest, TestI420Frames) {
   const int kNumFrame = 4;
   ScopedVector<I420VideoFrame> copied_input_frames;
-  std::vector<uint8_t*> ybuffer_pointers;
+  std::vector<const uint8_t*> ybuffer_pointers;
   for (int i = 0; i < kNumFrame; ++i) {
     input_frames_.push_back(CreateI420VideoFrame(static_cast<uint8_t>(i + 1)));
-    ybuffer_pointers.push_back(input_frames_[i]->buffer(kYPlane));
+    const I420VideoFrame* const_input_frame = input_frames_[i];
+    ybuffer_pointers.push_back(const_input_frame->buffer(kYPlane));
     // Copy input frames because the buffer data will be swapped.
     copied_input_frames.push_back(input_frames_[i]->CloneFrame());
     AddInputFrame(input_frames_[i]);