Revert "I420VideoFrame: Remove functions set_width, set_height, and ResetSize"
This reverts commit r8434.
Reason for revert: Introduced a race condition. If ViECaptureProcess() -> SwapCapturedAndDeliverFrameIfAvailable() is called twice without a call to OnIncomingCapturedFrame() in between (with both captured_frame_ and deliver_frame_ populated), an old frame will be delivered again, since captured_frame_->IsZeroSize() will never be true.
BUG=4352
TBR=perkj@webrtc.org, stefan@webrtc.org, tommi@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/40129004
Cr-Commit-Position: refs/heads/master@{#8530}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8530 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/common_video/i420_video_frame.cc b/webrtc/common_video/i420_video_frame.cc
index b2ba158..6924708 100644
--- a/webrtc/common_video/i420_video_frame.cc
+++ b/webrtc/common_video/i420_video_frame.cc
@@ -151,8 +151,33 @@
return -1;
}
+int I420VideoFrame::set_width(int width) {
+ if (CheckDimensions(width, height_,
+ y_plane_.stride(), u_plane_.stride(),
+ v_plane_.stride()) < 0)
+ return -1;
+ width_ = width;
+ return 0;
+}
+
+int I420VideoFrame::set_height(int height) {
+ if (CheckDimensions(width_, height,
+ y_plane_.stride(), u_plane_.stride(),
+ v_plane_.stride()) < 0)
+ return -1;
+ height_ = height;
+ return 0;
+}
+
bool I420VideoFrame::IsZeroSize() const {
- return width() == 0 || height() == 0;
+ return (y_plane_.IsZeroSize() && u_plane_.IsZeroSize() &&
+ v_plane_.IsZeroSize());
+}
+
+void I420VideoFrame::ResetSize() {
+ y_plane_.ResetSize();
+ u_plane_.ResetSize();
+ v_plane_.ResetSize();
}
void* I420VideoFrame::native_handle() const { return NULL; }
diff --git a/webrtc/common_video/i420_video_frame_unittest.cc b/webrtc/common_video/i420_video_frame_unittest.cc
index 47017ef..2b96634 100644
--- a/webrtc/common_video/i420_video_frame_unittest.cc
+++ b/webrtc/common_video/i420_video_frame_unittest.cc
@@ -44,8 +44,13 @@
TEST(TestI420VideoFrame, WidthHeightValues) {
I420VideoFrame frame;
const int valid_value = 10;
+ const int invalid_value = -1;
EXPECT_EQ(0, frame.CreateEmptyFrame(10, 10, 10, 14, 90));
EXPECT_EQ(valid_value, frame.width());
+ EXPECT_EQ(invalid_value, frame.set_width(invalid_value));
+ EXPECT_EQ(valid_value, frame.height());
+ EXPECT_EQ(valid_value, frame.height());
+ EXPECT_EQ(invalid_value, frame.set_height(0));
EXPECT_EQ(valid_value, frame.height());
frame.set_timestamp(123u);
EXPECT_EQ(123u, frame.timestamp());
@@ -71,6 +76,14 @@
frame.allocated_size(kVPlane));
}
+TEST(TestI420VideoFrame, ResetSize) {
+ I420VideoFrame frame;
+ EXPECT_EQ(0, frame. CreateEmptyFrame(10, 10, 12, 14, 220));
+ EXPECT_FALSE(frame.IsZeroSize());
+ frame.ResetSize();
+ EXPECT_TRUE(frame.IsZeroSize());
+}
+
TEST(TestI420VideoFrame, CopyFrame) {
uint32_t timestamp = 1;
int64_t ntp_time_ms = 2;
diff --git a/webrtc/common_video/interface/texture_video_frame.h b/webrtc/common_video/interface/texture_video_frame.h
index 225a0ec..9a1fee0 100644
--- a/webrtc/common_video/interface/texture_video_frame.h
+++ b/webrtc/common_video/interface/texture_video_frame.h
@@ -68,6 +68,7 @@
virtual int allocated_size(PlaneType type) const OVERRIDE;
virtual int stride(PlaneType type) const OVERRIDE;
virtual bool IsZeroSize() const OVERRIDE;
+ virtual void ResetSize() OVERRIDE;
virtual void* native_handle() const OVERRIDE;
protected:
diff --git a/webrtc/common_video/texture_video_frame.cc b/webrtc/common_video/texture_video_frame.cc
index 7772f12..ce6a040 100644
--- a/webrtc/common_video/texture_video_frame.cc
+++ b/webrtc/common_video/texture_video_frame.cc
@@ -20,8 +20,8 @@
uint32_t timestamp,
int64_t render_time_ms)
: handle_(handle) {
- width_ = width;
- height_ = height;
+ set_width(width);
+ set_height(height);
set_timestamp(timestamp);
set_render_time_ms(render_time_ms);
}
@@ -107,6 +107,10 @@
return true;
}
+void TextureVideoFrame::ResetSize() {
+ assert(false); // Should not be called.
+}
+
void* TextureVideoFrame::native_handle() const { return handle_.get(); }
int TextureVideoFrame::CheckDimensions(
diff --git a/webrtc/common_video/texture_video_frame_unittest.cc b/webrtc/common_video/texture_video_frame_unittest.cc
index c8c21c4..f574833 100644
--- a/webrtc/common_video/texture_video_frame_unittest.cc
+++ b/webrtc/common_video/texture_video_frame_unittest.cc
@@ -40,6 +40,10 @@
EXPECT_EQ(10, frame.render_time_ms());
EXPECT_EQ(&handle, frame.native_handle());
+ EXPECT_EQ(0, frame.set_width(320));
+ EXPECT_EQ(320, frame.width());
+ EXPECT_EQ(0, frame.set_height(240));
+ EXPECT_EQ(240, frame.height());
frame.set_timestamp(200);
EXPECT_EQ(200u, frame.timestamp());
frame.set_render_time_ms(20);
diff --git a/webrtc/modules/utility/source/file_player_impl.cc b/webrtc/modules/utility/source/file_player_impl.cc
index dc30d0a..5d935fb 100644
--- a/webrtc/modules/utility/source/file_player_impl.cc
+++ b/webrtc/modules/utility/source/file_player_impl.cc
@@ -511,6 +511,7 @@
// No new video data read from file.
if(_encodedData.payloadSize == 0)
{
+ videoFrame.ResetSize();
return -1;
}
int32_t retVal = 0;
diff --git a/webrtc/modules/utility/source/video_coder.cc b/webrtc/modules/utility/source/video_coder.cc
index 78a99f0..957826c 100644
--- a/webrtc/modules/utility/source/video_coder.cc
+++ b/webrtc/modules/utility/source/video_coder.cc
@@ -64,6 +64,7 @@
int32_t VideoCoder::Decode(I420VideoFrame& decodedVideo,
const EncodedVideoData& encodedData)
{
+ decodedVideo.ResetSize();
if(encodedData.payloadSize <= 0)
{
return -1;
diff --git a/webrtc/modules/utility/source/video_frames_queue.cc b/webrtc/modules/utility/source/video_frames_queue.cc
index d5077d8..9ade8b5 100644
--- a/webrtc/modules/utility/source/video_frames_queue.cc
+++ b/webrtc/modules/utility/source/video_frames_queue.cc
@@ -93,7 +93,10 @@
// No need to reuse texture frames because they do not allocate memory.
if (ptrOldFrame->native_handle() == NULL) {
ptrOldFrame->set_timestamp(0);
+ ptrOldFrame->set_width(0);
+ ptrOldFrame->set_height(0);
ptrOldFrame->set_render_time_ms(0);
+ ptrOldFrame->ResetSize();
_emptyFrames.push_back(ptrOldFrame);
} else {
delete ptrOldFrame;
diff --git a/webrtc/modules/video_coding/main/source/video_sender_unittest.cc b/webrtc/modules/video_coding/main/source/video_sender_unittest.cc
index 6666e4b..a59d0c0 100644
--- a/webrtc/modules/video_coding/main/source/video_sender_unittest.cc
+++ b/webrtc/modules/video_coding/main/source/video_sender_unittest.cc
@@ -71,12 +71,12 @@
class EmptyFrameGenerator : public FrameGenerator {
public:
virtual I420VideoFrame* NextFrame() OVERRIDE {
- frame_.reset(new I420VideoFrame());
- return frame_.get();
+ frame_.ResetSize();
+ return &frame_;
}
private:
- rtc::scoped_ptr<I420VideoFrame> frame_;
+ I420VideoFrame frame_;
};
class PacketizationCallback : public VCMPacketizationCallback {
diff --git a/webrtc/modules/video_processing/main/test/unit_test/video_processing_unittest.cc b/webrtc/modules/video_processing/main/test/unit_test/video_processing_unittest.cc
index 6c77911..70e19cc 100644
--- a/webrtc/modules/video_processing/main/test/unit_test/video_processing_unittest.cc
+++ b/webrtc/modules/video_processing/main/test/unit_test/video_processing_unittest.cc
@@ -91,6 +91,8 @@
VideoProcessingModule::FrameStats stats;
// Video frame with unallocated buffer.
I420VideoFrame videoFrame;
+ videoFrame.set_width(width_);
+ videoFrame.set_height(height_);
EXPECT_EQ(-3, vpm_->GetFrameStats(&stats, videoFrame));
@@ -118,21 +120,21 @@
TEST_F(VideoProcessingModuleTest, HandleBadSize) {
VideoProcessingModule::FrameStats stats;
- I420VideoFrame bad_frame;
- bad_frame.CreateEmptyFrame(width_, 0, width_, (width_ + 1) / 2,
- (width_ + 1) / 2);
- EXPECT_EQ(-3, vpm_->GetFrameStats(&stats, bad_frame));
+ video_frame_.ResetSize();
+ video_frame_.set_width(width_);
+ video_frame_.set_height(0);
+ EXPECT_EQ(-3, vpm_->GetFrameStats(&stats, video_frame_));
- EXPECT_EQ(-1, vpm_->ColorEnhancement(&bad_frame));
+ EXPECT_EQ(-1, vpm_->ColorEnhancement(&video_frame_));
- EXPECT_EQ(-1, vpm_->Deflickering(&bad_frame, &stats));
+ EXPECT_EQ(-1, vpm_->Deflickering(&video_frame_, &stats));
- EXPECT_EQ(-3, vpm_->BrightnessDetection(bad_frame, stats));
+ EXPECT_EQ(-3, vpm_->BrightnessDetection(video_frame_, stats));
EXPECT_EQ(VPM_PARAMETER_ERROR, vpm_->SetTargetResolution(0,0,0));
I420VideoFrame *out_frame = NULL;
- EXPECT_EQ(VPM_PARAMETER_ERROR, vpm_->PreprocessFrame(bad_frame,
+ EXPECT_EQ(VPM_PARAMETER_ERROR, vpm_->PreprocessFrame(video_frame_,
&out_frame));
}
@@ -333,10 +335,8 @@
int cropped_width,
int cropped_height,
I420VideoFrame* cropped_frame) {
- cropped_frame->CreateEmptyFrame(cropped_width, cropped_height,
- cropped_width,
- (cropped_width + 1) / 2,
- (cropped_width + 1) / 2);
+ cropped_frame->set_width(cropped_width);
+ cropped_frame->set_height(cropped_height);
EXPECT_EQ(0,
ConvertToI420(kI420, source_data, offset_x, offset_y, source_width,
source_height, 0, kRotateNone, cropped_frame));
diff --git a/webrtc/modules/video_render/video_render_frames.cc b/webrtc/modules/video_render/video_render_frames.cc
index 3df4ea4..d790877 100644
--- a/webrtc/modules/video_render/video_render_frames.cc
+++ b/webrtc/modules/video_render/video_render_frames.cc
@@ -125,6 +125,7 @@
int32_t VideoRenderFrames::ReturnFrame(I420VideoFrame* old_frame) {
// No need to reuse texture frames because they do not allocate memory.
if (old_frame->native_handle() == NULL) {
+ old_frame->ResetSize();
old_frame->set_timestamp(0);
old_frame->set_render_time_ms(0);
empty_frames_.push_back(old_frame);
diff --git a/webrtc/video_engine/vie_capturer.cc b/webrtc/video_engine/vie_capturer.cc
index 0134c32..06f85df 100644
--- a/webrtc/video_engine/vie_capturer.cc
+++ b/webrtc/video_engine/vie_capturer.cc
@@ -632,6 +632,7 @@
if (deliver_frame_ == NULL)
deliver_frame_.reset(new I420VideoFrame());
deliver_frame_->SwapFrame(captured_frame_.get());
+ captured_frame_->ResetSize();
return true;
}
diff --git a/webrtc/video_frame.h b/webrtc/video_frame.h
index 80b1582..2dd7c61 100644
--- a/webrtc/video_frame.h
+++ b/webrtc/video_frame.h
@@ -111,6 +111,12 @@
// Get allocated stride per plane.
virtual int stride(PlaneType type) const;
+ // Set frame width.
+ virtual int set_width(int width);
+
+ // Set frame height.
+ virtual int set_height(int height);
+
// Get frame width.
virtual int width() const { return width_; }
@@ -157,6 +163,10 @@
// Return true if underlying plane buffers are of zero size, false if not.
virtual bool IsZeroSize() const;
+ // Reset underlying plane buffers sizes to 0. This function doesn't
+ // clear memory.
+ virtual void ResetSize();
+
// Return the handle of the underlying video frame. This is used when the
// frame is backed by a texture. The object should be destroyed when it is no
// longer in use, so the underlying resource can be freed.
@@ -170,8 +180,6 @@
int stride_y,
int stride_u,
int stride_v);
- int width_;
- int height_;
private:
// Get the pointer to a specific plane.
@@ -182,6 +190,8 @@
Plane y_plane_;
Plane u_plane_;
Plane v_plane_;
+ int width_;
+ int height_;
uint32_t timestamp_;
int64_t ntp_time_ms_;
int64_t render_time_ms_;