DCHECK frame parameters instead of return codes.

We should never be creating video frames without width/height. If these
DCHECKs fire we should be fixing the calling code instead.

BUG=4359
R=magjed@webrtc.org, tommi@webrtc.org

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

Cr-Commit-Position: refs/heads/master@{#8779}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8779 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
pbos@webrtc.org 2015-03-18 14:11:39 +00:00
parent 4346d92578
commit 5a477a0bc6
4 changed files with 16 additions and 35 deletions

View File

@ -47,15 +47,20 @@ I420VideoFrame::I420VideoFrame(NativeHandle* handle,
ntp_time_ms_(0),
render_time_ms_(render_time_ms),
rotation_(kVideoRotation_0) {
DCHECK(handle != nullptr);
DCHECK_GT(width, 0);
DCHECK_GT(height, 0);
}
int I420VideoFrame::CreateEmptyFrame(int width, int height,
int stride_y, int stride_u, int stride_v) {
const int half_width = (width + 1) / 2;
if (width <= 0 || height <= 0 || stride_y < width || stride_u < half_width ||
stride_v < half_width) {
return -1;
}
DCHECK_GT(width, 0);
DCHECK_GT(height, 0);
DCHECK_GE(stride_y, width);
DCHECK_GE(stride_u, half_width);
DCHECK_GE(stride_v, half_width);
// Creating empty frame - reset all values.
timestamp_ = 0;
ntp_time_ms_ = 0;
@ -105,8 +110,7 @@ int I420VideoFrame::CreateFrame(const uint8_t* buffer_y,
const int expected_size_y = height * stride_y;
const int expected_size_u = half_height * stride_u;
const int expected_size_v = half_height * stride_v;
if (CreateEmptyFrame(width, height, stride_y, stride_u, stride_v) < 0)
return -1;
CreateEmptyFrame(width, height, stride_y, stride_u, stride_v);
memcpy(buffer(kYPlane), buffer_y, expected_size_y);
memcpy(buffer(kUPlane), buffer_u, expected_size_u);
memcpy(buffer(kVPlane), buffer_v, expected_size_v);
@ -145,7 +149,9 @@ void I420VideoFrame::ShallowCopy(const I420VideoFrame& videoFrame) {
I420VideoFrame* I420VideoFrame::CloneFrame() const {
rtc::scoped_ptr<I420VideoFrame> new_frame(new I420VideoFrame());
if (new_frame->CopyFrame(*this) == -1) {
// CopyFrame failed.
// TODO(pbos): Make void, not war.
// CopyFrame failed this shouldn't happen.
RTC_NOTREACHED();
return NULL;
}
return new_frame.release();

View File

@ -42,16 +42,8 @@ int ExpectedSize(int plane_stride, int image_height, PlaneType type);
TEST(TestI420VideoFrame, InitialValues) {
I420VideoFrame frame;
// Invalid arguments - one call for each variable.
EXPECT_TRUE(frame.IsZeroSize());
EXPECT_EQ(kVideoRotation_0, frame.rotation());
EXPECT_EQ(-1, frame.CreateEmptyFrame(0, 10, 10, 14, 14));
EXPECT_EQ(-1, frame.CreateEmptyFrame(10, -1, 10, 90, 14));
EXPECT_EQ(-1, frame.CreateEmptyFrame(10, 10, 0, 14, 18));
EXPECT_EQ(-1, frame.CreateEmptyFrame(10, 10, 10, -2, 13));
EXPECT_EQ(-1, frame.CreateEmptyFrame(10, 10, 10, 14, 0));
EXPECT_EQ(0, frame.CreateEmptyFrame(10, 10, 10, 14, 90));
EXPECT_FALSE(frame.IsZeroSize());
}
TEST(TestI420VideoFrame, WidthHeightValues) {

View File

@ -114,26 +114,6 @@ TEST_F(VideoProcessingModuleTest, HandleBadStats) {
EXPECT_EQ(-3, vpm_->BrightnessDetection(video_frame_, stats));
}
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));
EXPECT_EQ(-1, vpm_->ColorEnhancement(&bad_frame));
EXPECT_EQ(-1, vpm_->Deflickering(&bad_frame, &stats));
EXPECT_EQ(-3, vpm_->BrightnessDetection(bad_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, &out_frame));
}
TEST_F(VideoProcessingModuleTest, IdenticalResultsAfterReset) {
I420VideoFrame video_frame2;
VideoProcessingModule::FrameStats stats;

View File

@ -32,6 +32,9 @@ class I420VideoFrame {
uint32_t timestamp,
int64_t render_time_ms);
// TODO(pbos): Make all create/copy functions void, they should not be able to
// fail (which should be DCHECK/CHECKed instead).
// CreateEmptyFrame: Sets frame dimensions and allocates buffers based
// on set dimensions - height and plane stride.
// If required size is bigger than the allocated one, new buffers of adequate