Make cricket::VideoFrame inherit webrtc::VideoFrame.

webrtc/media/engine/webrtcvideoframe.h

Index: webrtc/media/engine/webrtcvideoframe.h
diff --git a/webrtc/media/engine/webrtcvideoframe.h b/webrtc/media/engine/webrtcvideoframe.h
index 79ad571236cfd922898989fd453f461daecc12db..c2ab07824f9a761af845926092ba59124f73858d 100644
--- a/webrtc/media/engine/webrtcvideoframe.h
+++ b/webrtc/media/engine/webrtcvideoframe.h
@@ -8,6 +8,10 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
+// Deprecated, replace by cricket::WebRtcVideoFrame by

[perkj_webrtc, 2016/10/04 15:36:24]

/s replace....

[pthatcher1, 2016/10/04 15:41:54]

Same here.  (one "with" instead of two "by"s)

[nisse-webrtc, 2016/10/06 07:49:44]

Done.

+// webrtc::VideoFrame everywhere, then delete this file. See
+// https://bugs.chromium.org/p/webrtc/issues/detail?id=5682.
+
 #ifndef WEBRTC_MEDIA_ENGINE_WEBRTCVIDEOFRAME_H_
 #define WEBRTC_MEDIA_ENGINE_WEBRTCVIDEOFRAME_H_
 
@@ -22,100 +26,21 @@
 
 namespace cricket {
 
-// TODO(nisse): This class will be deleted when the cricket::VideoFrame and
-// webrtc::VideoFrame classes are merged. See
-// https://bugs.chromium.org/p/webrtc/issues/detail?id=5682. Try to use only the
-// preferred constructor, and the non-deprecated methods of the VideoFrame base
-// class.
 class WebRtcVideoFrame : public VideoFrame {
  public:
-  // TODO(nisse): Deprecated. Using the default constructor violates the
-  // reasonable assumption that video_frame_buffer() returns a valid buffer.
-  WebRtcVideoFrame();
-
-  // Preferred constructor.
+  WebRtcVideoFrame() : VideoFrame() {}
   WebRtcVideoFrame(const rtc::scoped_refptr<webrtc::VideoFrameBuffer>& buffer,
                    webrtc::VideoRotation rotation,
-                   int64_t timestamp_us,
-                   uint32_t transport_frame_id);
-
-  // Alternative constructor, when not knowing or caring about the
-  // transport_frame_id. Which is set to zero.
+                   int64_t timestamp_us)
+      : VideoFrame(buffer, rotation, timestamp_us) {}
   WebRtcVideoFrame(const rtc::scoped_refptr<webrtc::VideoFrameBuffer>& buffer,
                    webrtc::VideoRotation rotation,
-                   int64_t timestamp_us);
-
-  // TODO(nisse): Deprecated, delete as soon as all callers have switched to the
-  // above constructor with microsecond timestamp.
-  WebRtcVideoFrame(const rtc::scoped_refptr<webrtc::VideoFrameBuffer>& buffer,
-                   int64_t timestamp_ns,
-                   webrtc::VideoRotation rotation);
-
-  ~WebRtcVideoFrame();
-
-  // TODO(nisse): Init (and its helpers Reset and Validate) are used
-  // only by the LoadFrame function used in the VideoFrame unittests.
-  // Rewrite tests, and delete this function.
-
-  // Creates a frame from a raw sample with FourCC "format" and size "w" x "h".
-  // "h" can be negative indicating a vertically flipped image.
-  // "dh" is destination height if cropping is desired and is always positive.
-  // Returns "true" if successful.
-  bool Init(uint32_t format,
-            int w,
-            int h,
-            int dw,
-            int dh,
-            uint8_t* sample,
-            size_t sample_size,
-            int64_t timestamp_ns,
-            webrtc::VideoRotation rotation);
-
-  void InitToEmptyBuffer(int w, int h);
-
-  int width() const override;
-  int height() const override;
-
-  const rtc::scoped_refptr<webrtc::VideoFrameBuffer>& video_frame_buffer()
-      const override;
-
-  uint32_t transport_frame_id() const override;
-
-  int64_t timestamp_us() const override;
-  void set_timestamp_us(int64_t time_us) override;
-
-  webrtc::VideoRotation rotation() const override;
-
- protected:
-  // Creates a frame from a raw sample with FourCC |format| and size |w| x |h|.
-  // |h| can be negative indicating a vertically flipped image.
-  // |dw| is destination width; can be less than |w| if cropping is desired.
-  // |dh| is destination height, like |dw|, but must be a positive number.
-  // Returns whether the function succeeded or failed.
-  bool Reset(uint32_t format,
-             int w,
-             int h,
-             int dw,
-             int dh,
-             uint8_t* sample,
-             size_t sample_size,
-             int64_t timestamp_us,
-             webrtc::VideoRotation rotation,
-             bool apply_rotation);
-
- private:
-  // Tests mutate |rotation_|, so the base test class is a friend.
-  friend class WebRtcVideoFrameTest;
-
-  // An opaque reference counted handle that stores the pixel data.
-  rtc::scoped_refptr<webrtc::VideoFrameBuffer> video_frame_buffer_;
-  int64_t timestamp_us_;
-  uint32_t transport_frame_id_;
-  webrtc::VideoRotation rotation_;
-
-  // This is mutable as the calculation is expensive but once calculated, it
-  // remains const.
-  mutable std::unique_ptr<VideoFrame> rotated_frame_;
+                   int64_t timestamp_us,
+                   uint32_t transport_frame_id)
+      : VideoFrame(buffer, rotation, timestamp_us) {
+    // For now, transport_frame_id and rtp timestamp are the same.
+    set_timestamp(transport_frame_id);

[pthatcher1, 2016/10/04 15:41:54]

That seems like kind of a crazy assumption.  Would it be better to have a
separate field in the class?

[nisse-webrtc, 2016/10/06 07:49:44]

That's how it currently works. I discussed it with sergeyu (who wrote the code
using transport_frame_id), and we agreed it's good enough for the time being. I
could add a new field, but I really dislike adding even more timestamps to
webrtc::VideoFrame.

Unclear when we're get to cleaning this up (since I'm going to focus on quic
coming months), but I'd like to delete both the timestamp field (90 kHz rtp
clock) and the ntp timestamp from the webrtc::VideoFrame class. Left would only
be timestamp_us (rtc::TimeMicros time) and (if needed) transport_frame_id, which
happens to be the same thing as the rtp timestamp.

If Per agrees, I can add a separate webrtc::VideoFrame::transport_frame_id_
member.

[pthatcher2, 2016/10/06 16:43:50]

I'll leave it up to you guys to decide.

+  }
 };
 
 }  // namespace cricket