Issue 4366: Adapted frames have wrong width and height and are cropped.
When a frame being stretched, the original rotation information is lost. This is to ensure it's carried over.
Also removed StretchToBuffer function as it's not called and dangerous.
BUG=4366
R=pthatcher@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/51869004
Cr-Commit-Position: refs/heads/master@{#9224}
diff --git a/talk/libjingle_tests.gyp b/talk/libjingle_tests.gyp
index 3ca2f1f..80cd1fa 100755
--- a/talk/libjingle_tests.gyp
+++ b/talk/libjingle_tests.gyp
@@ -103,6 +103,7 @@
'media/webrtc/webrtcvideocapturer_unittest.cc',
'media/base/videoframe_unittest.h',
'media/webrtc/webrtcvideoframe_unittest.cc',
+ 'media/webrtc/webrtcvideoframefactory_unittest.cc',
# Disabled because some tests fail.
# TODO(ronghuawu): Reenable these tests.
diff --git a/talk/media/base/videoframe.cc b/talk/media/base/videoframe.cc
index 79f52fc..7e79bf6 100644
--- a/talk/media/base/videoframe.cc
+++ b/talk/media/base/videoframe.cc
@@ -190,28 +190,6 @@
static_cast<int>(width), static_cast<int>(height), interpolate);
}
-size_t VideoFrame::StretchToBuffer(size_t dst_width, size_t dst_height,
- uint8* dst_buffer, size_t size,
- bool interpolate, bool vert_crop) const {
- if (!dst_buffer) {
- LOG(LS_ERROR) << "NULL dst_buffer pointer.";
- return 0;
- }
-
- size_t needed = SizeOf(dst_width, dst_height);
- if (needed <= size) {
- uint8* dst_y = dst_buffer;
- uint8* dst_u = dst_y + dst_width * dst_height;
- uint8* dst_v = dst_u + ((dst_width + 1) >> 1) * ((dst_height + 1) >> 1);
- StretchToPlanes(dst_y, dst_u, dst_v,
- static_cast<int32>(dst_width),
- static_cast<int32>((dst_width + 1) >> 1),
- static_cast<int32>((dst_width + 1) >> 1),
- dst_width, dst_height, interpolate, vert_crop);
- }
- return needed;
-}
-
void VideoFrame::StretchToFrame(VideoFrame* dst,
bool interpolate, bool vert_crop) const {
if (!dst) {
@@ -225,6 +203,8 @@
interpolate, vert_crop);
dst->SetElapsedTime(GetElapsedTime());
dst->SetTimeStamp(GetTimeStamp());
+ // Stretched frame should have the same rotation as the source.
+ dst->SetRotation(GetVideoRotation());
}
VideoFrame* VideoFrame::Stretch(size_t dst_width, size_t dst_height,
diff --git a/talk/media/base/videoframe.h b/talk/media/base/videoframe.h
index 0aac645..ac53ca2 100644
--- a/talk/media/base/videoframe.h
+++ b/talk/media/base/videoframe.h
@@ -188,16 +188,6 @@
uint8 *y, uint8 *u, uint8 *v, int32 pitchY, int32 pitchU, int32 pitchV,
size_t width, size_t height, bool interpolate, bool crop) const;
- // Writes the frame into the given frame buffer, stretched to the given width
- // and height, provided that it is of sufficient size. Returns the frame's
- // actual size, regardless of whether it was written or not (like snprintf).
- // If there is insufficient space, nothing is written. The parameter
- // "interpolate" controls whether to interpolate or just take the
- // nearest-point. The parameter "crop" controls whether to crop this frame to
- // the aspect ratio of the given dimensions before stretching.
- virtual size_t StretchToBuffer(size_t w, size_t h, uint8 *buffer, size_t size,
- bool interpolate, bool crop) const;
-
// Writes the frame into the target VideoFrame, stretched to the size of that
// frame. The parameter "interpolate" controls whether to interpolate or just
// take the nearest-point. The parameter "crop" controls whether to crop this
@@ -230,6 +220,7 @@
size_t pixel_height,
int64_t elapsed_time,
int64_t time_stamp) const = 0;
+ virtual void SetRotation(webrtc::VideoRotation rotation) = 0;
};
} // namespace cricket
diff --git a/talk/media/webrtc/webrtcvideoframe.h b/talk/media/webrtc/webrtcvideoframe.h
index 06ac52b..7900472 100644
--- a/talk/media/webrtc/webrtcvideoframe.h
+++ b/talk/media/webrtc/webrtcvideoframe.h
@@ -131,7 +131,9 @@
const VideoFrame* GetCopyWithRotationApplied() const override;
protected:
- void SetRotation(webrtc::VideoRotation rotation) { rotation_ = rotation; }
+ void SetRotation(webrtc::VideoRotation rotation) override {
+ rotation_ = rotation;
+ }
private:
virtual VideoFrame* CreateEmptyFrame(int w, int h, size_t pixel_width,
diff --git a/talk/media/webrtc/webrtcvideoframefactory_unittest.cc b/talk/media/webrtc/webrtcvideoframefactory_unittest.cc
new file mode 100644
index 0000000..4dbad05
--- /dev/null
+++ b/talk/media/webrtc/webrtcvideoframefactory_unittest.cc
@@ -0,0 +1,125 @@
+/*
+ * libjingle
+ * Copyright 2015 Google Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote products
+ * derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
+ * EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
+ * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <string.h>
+
+#include "talk/media/base/videoframe_unittest.h"
+#include "talk/media/webrtc/webrtcvideoframe.h"
+#include "talk/media/webrtc/webrtcvideoframefactory.h"
+
+class WebRtcVideoFrameFactoryTest
+ : public VideoFrameTest<cricket::WebRtcVideoFrameFactory> {
+ public:
+ WebRtcVideoFrameFactoryTest() {}
+
+ void InitFrame(webrtc::VideoRotation frame_rotation) {
+ const int frame_width = 1920;
+ const int frame_height = 1080;
+
+ // Build the CapturedFrame.
+ captured_frame_.fourcc = cricket::FOURCC_I420;
+ captured_frame_.pixel_width = 1;
+ captured_frame_.pixel_height = 1;
+ captured_frame_.elapsed_time = 1234;
+ captured_frame_.time_stamp = 5678;
+ captured_frame_.rotation = frame_rotation;
+ captured_frame_.width = frame_width;
+ captured_frame_.height = frame_height;
+ captured_frame_.data_size =
+ (frame_width * frame_height) +
+ ((frame_width + 1) / 2) * ((frame_height + 1) / 2) * 2;
+ captured_frame_buffer_.reset(new uint8[captured_frame_.data_size]);
+ // Initialize memory to satisfy DrMemory tests.
+ memset(captured_frame_buffer_.get(), 0, captured_frame_.data_size);
+ captured_frame_.data = captured_frame_buffer_.get();
+ }
+
+ void VerifyFrame(cricket::VideoFrame* dest_frame,
+ webrtc::VideoRotation src_rotation,
+ int src_width,
+ int src_height,
+ bool apply_rotation) {
+ if (!apply_rotation) {
+ EXPECT_EQ(dest_frame->GetRotation(), src_rotation);
+ EXPECT_EQ(dest_frame->GetWidth(), src_width);
+ EXPECT_EQ(dest_frame->GetHeight(), src_height);
+ } else {
+ EXPECT_EQ(dest_frame->GetRotation(), webrtc::kVideoRotation_0);
+ if (src_rotation == webrtc::kVideoRotation_90 ||
+ src_rotation == webrtc::kVideoRotation_270) {
+ EXPECT_EQ(dest_frame->GetWidth(), src_height);
+ EXPECT_EQ(dest_frame->GetHeight(), src_width);
+ } else {
+ EXPECT_EQ(dest_frame->GetWidth(), src_width);
+ EXPECT_EQ(dest_frame->GetHeight(), src_height);
+ }
+ }
+ }
+
+ void TestCreateAliasedFrame(bool apply_rotation) {
+ cricket::VideoFrameFactory& factory = factory_;
+ factory.SetApplyRotation(apply_rotation);
+ InitFrame(webrtc::kVideoRotation_270);
+ const cricket::CapturedFrame& captured_frame = get_captured_frame();
+ // Create the new frame from the CapturedFrame.
+ rtc::scoped_ptr<cricket::VideoFrame> frame;
+ int new_width = captured_frame.width / 2;
+ int new_height = captured_frame.height / 2;
+ frame.reset(factory.CreateAliasedFrame(&captured_frame, new_width,
+ new_height, new_width, new_height));
+ VerifyFrame(frame.get(), webrtc::kVideoRotation_270, new_width, new_height,
+ apply_rotation);
+
+ frame.reset(factory.CreateAliasedFrame(
+ &captured_frame, new_width, new_height, new_width / 2, new_height / 2));
+ VerifyFrame(frame.get(), webrtc::kVideoRotation_270, new_width / 2,
+ new_height / 2, apply_rotation);
+
+ // Reset the frame first so it's exclusive hence we could go through the
+ // StretchToFrame code path in CreateAliasedFrame.
+ frame.reset();
+ frame.reset(factory.CreateAliasedFrame(
+ &captured_frame, new_width, new_height, new_width / 2, new_height / 2));
+ VerifyFrame(frame.get(), webrtc::kVideoRotation_270, new_width / 2,
+ new_height / 2, apply_rotation);
+ }
+
+ const cricket::CapturedFrame& get_captured_frame() { return captured_frame_; }
+
+ private:
+ cricket::CapturedFrame captured_frame_;
+ rtc::scoped_ptr<uint8[]> captured_frame_buffer_;
+ cricket::WebRtcVideoFrameFactory factory_;
+};
+
+TEST_F(WebRtcVideoFrameFactoryTest, NoApplyRotation) {
+ TestCreateAliasedFrame(false);
+}
+
+TEST_F(WebRtcVideoFrameFactoryTest, ApplyRotation) {
+ TestCreateAliasedFrame(true);
+}
