From 50b229509187cf63b5c80ff5ae55694f0e84ee23 Mon Sep 17 00:00:00 2001
From: "magjed@webrtc.org" <magjed@webrtc.org>
Date: Mon, 2 Mar 2015 10:03:47 +0000
Subject: [PATCH] cricket::VideoFrameFactory: Don't overwrite frames in use

VideoFrameFactory has a single frame buffer that is used when scaling frames. If the previous frame is still in use, we need to allocate a new frame.

BUG=4347
R=perkj@webrtc.org

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

Cr-Commit-Position: refs/heads/master@{#8549}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8549 4adac7df-926f-26a2-2b94-8c16560cd09d
---
 talk/media/base/videoframe.h          | 11 ++++++++---
 talk/media/base/videoframefactory.cc  |  6 +++---
 talk/media/webrtc/webrtcvideoframe.cc |  8 ++++++++
 talk/media/webrtc/webrtcvideoframe.h  |  1 +
 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/talk/media/base/videoframe.h b/talk/media/base/videoframe.h
index 56621ad5a4..2e9a01bfc1 100644
--- a/talk/media/base/videoframe.h
+++ b/talk/media/base/videoframe.h
@@ -132,9 +132,14 @@ class VideoFrame {
   virtual VideoFrame *Copy() const = 0;
 
   // Since VideoFrame supports shallow copy and the internal frame buffer might
-  // be shared, in case VideoFrame needs exclusive access of the frame buffer,
-  // user can call MakeExclusive() to make sure the frame buffer is exclusive
-  // accessable to the current object.  This might mean a deep copy of the frame
+  // be shared, this function can be used to check exclusive ownership. The
+  // default implementation is conservative and returns false. Subclasses with
+  // knowledge of implementation specific details can override this.
+  virtual bool IsExclusive() const { return false; }
+
+  // In case VideoFrame needs exclusive access of the frame buffer, user can
+  // call MakeExclusive() to make sure the frame buffer is exclusively
+  // accessible to the current object.  This might mean a deep copy of the frame
   // buffer if it is currently shared by other objects.
   virtual bool MakeExclusive() = 0;
 
diff --git a/talk/media/base/videoframefactory.cc b/talk/media/base/videoframefactory.cc
index b4725cdd8e..147f1faf81 100644
--- a/talk/media/base/videoframefactory.cc
+++ b/talk/media/base/videoframefactory.cc
@@ -46,9 +46,9 @@ VideoFrame* VideoFrameFactory::CreateAliasedFrame(
     return cropped_input_frame.release();
   }
 
-  // Create and stretch the output frame if it has not been created yet or its
-  // size is not same as the expected.
-  if (!output_frame_ ||
+  // Create and stretch the output frame if it has not been created yet, is
+  // still in use by others, or its size is not same as the expected.
+  if (!output_frame_ || !output_frame_->IsExclusive() ||
       output_frame_->GetWidth() != static_cast<size_t>(output_width) ||
       output_frame_->GetHeight() != static_cast<size_t>(output_height)) {
     output_frame_.reset(
diff --git a/talk/media/webrtc/webrtcvideoframe.cc b/talk/media/webrtc/webrtcvideoframe.cc
index 9764f31af5..0b1d4a8194 100644
--- a/talk/media/webrtc/webrtcvideoframe.cc
+++ b/talk/media/webrtc/webrtcvideoframe.cc
@@ -244,7 +244,15 @@ VideoFrame* WebRtcVideoFrame::Copy() const {
   return ret_val;
 }
 
+bool WebRtcVideoFrame::IsExclusive() const {
+  return video_buffer_->HasOneRef();
+}
+
 bool WebRtcVideoFrame::MakeExclusive() {
+  if (IsExclusive())
+    return true;
+
+  // Not exclusive already, need to copy.
   const size_t length = video_buffer_->length();
   RefCountedBuffer* exclusive_buffer = new RefCountedBuffer(length);
   memcpy(exclusive_buffer->data(), video_buffer_->data(), length);
diff --git a/talk/media/webrtc/webrtcvideoframe.h b/talk/media/webrtc/webrtcvideoframe.h
index 4de12b224c..2e34d67ee9 100644
--- a/talk/media/webrtc/webrtcvideoframe.h
+++ b/talk/media/webrtc/webrtcvideoframe.h
@@ -137,6 +137,7 @@ class WebRtcVideoFrame : public VideoFrame {
   }
 
   virtual VideoFrame* Copy() const;
+  virtual bool IsExclusive() const;
   virtual bool MakeExclusive();
   virtual size_t ConvertToRgbBuffer(uint32 to_fourcc, uint8* buffer,
                                     size_t size, int stride_rgb) const;