Cleanups in cricket::VideoFrame and cricket::WebRtcVideoFrame.

webrtc/media/engine/webrtcvideoframe.cc

Index: webrtc/media/engine/webrtcvideoframe.cc
diff --git a/webrtc/media/engine/webrtcvideoframe.cc b/webrtc/media/engine/webrtcvideoframe.cc
index 4f89c8b85d100562d14cb891c6b11e8f67f247d1..cbf21512ab589a83383601fa88118ecc9c6a93e3 100644
--- a/webrtc/media/engine/webrtcvideoframe.cc
+++ b/webrtc/media/engine/webrtcvideoframe.cc
@@ -149,26 +149,11 @@ bool WebRtcVideoFrame::Reset(uint32_t format,
   return true;
 }
 
-VideoFrame* WebRtcVideoFrame::CreateEmptyFrame(int w,
-                                               int h,
-                                               int64_t timestamp_us) const {
-  WebRtcVideoFrame* frame = new WebRtcVideoFrame();
-  frame->InitToEmptyBuffer(w, h, rtc::kNumNanosecsPerMicrosec * timestamp_us);
-  return frame;
-}
-
 void WebRtcVideoFrame::InitToEmptyBuffer(int w, int h) {
   video_frame_buffer_ = new rtc::RefCountedObject<webrtc::I420Buffer>(w, h);
   rotation_ = webrtc::kVideoRotation_0;
 }
 
-void WebRtcVideoFrame::InitToEmptyBuffer(int w, int h,
-                                         int64_t time_stamp_ns) {
-  video_frame_buffer_ = new rtc::RefCountedObject<webrtc::I420Buffer>(w, h);
-  SetTimeStamp(time_stamp_ns);
-  rotation_ = webrtc::kVideoRotation_0;
-}
-
 const VideoFrame* WebRtcVideoFrame::GetCopyWithRotationApplied() const {
   // If the frame is not rotated, the caller should reuse this frame instead of
   // making a redundant copy.
@@ -196,8 +181,9 @@ const VideoFrame* WebRtcVideoFrame::GetCopyWithRotationApplied() const {
     rotated_height = orig_width;
   }
 
-  rotated_frame_.reset(
-      CreateEmptyFrame(rotated_width, rotated_height, timestamp_us_));
+  rotated_frame_.reset(new WebRtcVideoFrame());

[nisse-webrtc, 2016/06/20 07:52:23]

The plan is to make VideoFrameBuffer always immutable. So it's more in line with
this plan to arrange this to first create an I420Buffer for the rotated image
(I420Buffer will stay mutable), and then use the WebRtcVideoFrame constructor
taking buffer, timestamp and rotation as arguments.

See https://bugs.chromium.org/p/webrtc/issues/detail?id=5921

[Sergey Ulanov, 2016/06/20 18:42:45]

Done.

[nisse-webrtc, 2016/06/21 07:52:35]

It's not quite enough; rotated_frame->video_frame_buffer() returns a reference
to the base class VideoFrameBuffer, where we'd like to delete the MutableData
methods. So I suggest you write it as

rtc::scoped_refptr<webrtc::I420Buffer> rotated_buffer(
  new RefCountedObject<webrtc::I420Buffer>(rotated_width, rotated_height));

libyuv::I420Rotate(... rotated_buffer->MutableDataY(), ...);

rotated_frame.reset(new WebRtcVideoFrame(rotated_buffer, ...);

(I find constructing the WebRtcVideoFrame last is logical, but order isn't that
important. The important detail is that mutation is done via a pointer to the
subclass I420Buffer, not the base class VideoFrameBuffer).

[Sergey Ulanov, 2016/06/21 17:25:55]

Done, though it's not really related to this CL, and potentially could be done
in a separate CL.
Also, we both agree that GetCopyWithRotationApplied() should be removed from
this class, so it doesn't makes sense to spend much time on it here.

+  rotated_frame_->InitToEmptyBuffer(rotated_width, rotated_height);
+  rotated_frame_->set_timestamp_us(timestamp_us_);
 
   // TODO(guoweis): Add a function in webrtc_libyuv.cc to convert from
   // VideoRotation to libyuv::RotationMode.