Skip to content

Commit 3011627

Browse files
nisseCommit bot
nisse
authored and
Commit bot
committed
Revert of Update test code to use I420Buffer when writing pixel data. (patchset #2 id:140001 of https://codereview.webrtc.org/2342783003/ )
Reason for revert: I was too impatient; this made android builds fail instead. See https://build.chromium.org/p/client.webrtc/builders/Linux32%20ARM/builds/585/steps/compile/logs/stdio Original issue's description: > Reland of Update test code to use I420Buffer when writing pixel data. (patchset #1 id:1 of https://codereview.webrtc.org/2342123003/ ) > > Reason for revert: > Intending to fix problem and reland. > > Original issue's description: > > Revert of Update test code to use I420Buffer when writing pixel data. (patchset #5 id:80001 of https://codereview.webrtc.org/2333373007/ ) > > > > Reason for revert: > > Fails 64-bit windows builds, it turns out I missed some of the needed int/size_t casts. Example https://build.chromium.org/p/client.webrtc/waterfall?builder=Win64%20Release > > > > Hope our windows try bots get back in working shape soon. > > > > Original issue's description: > > > Update test code to use I420Buffer when writing pixel data. > > > > > > VideoFrameBuffer and VideoFrame will become immutable. > > > > > > BUG=webrtc:5921 > > > [email protected], [email protected] > > > > > > Committed: https://crrev.com/280ad1514e44bf6717e5871526dd4632f759eb3d > > > Cr-Commit-Position: refs/heads/master@{#14249} > > > > [email protected],[email protected],[email protected] > > # Skipping CQ checks because original CL landed less than 1 days ago. > > NOPRESUBMIT=true > > NOTREECHECKS=true > > NOTRY=true > > BUG=webrtc:5921 > > > > Committed: https://crrev.com/fbf14607267adf03d235273283ca452a1e564861 > > Cr-Commit-Position: refs/heads/master@{#14251} > > [email protected],[email protected],[email protected] > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=webrtc:5921 > > Committed: https://crrev.com/d21534a8cfe636bbcf3d7bb151945590abc92b2a > Cr-Commit-Position: refs/heads/master@{#14258} [email protected],[email protected],[email protected] # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:5921 Review-Url: https://codereview.webrtc.org/2343083002 Cr-Commit-Position: refs/heads/master@{#14259}
1 parent d21534a commit 3011627

File tree

5 files changed

+85
-124
lines changed

5 files changed

+85
-124
lines changed

‎webrtc/common_video/libyuv/include/webrtc_libyuv.h‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,6 @@ int ExtractBuffer(const VideoFrame& input_frame, size_t size, uint8_t* buffer);
9494
// - dst_frame : Reference to a destination frame.
9595
// Return value: 0 if OK, < 0 otherwise.
9696

97-
// TODO(nisse): Deprecated, see
98-
// https://bugs.chromium.org/p/webrtc/issues/detail?id=5921.
9997
int ConvertToI420(VideoType src_video_type,
10098
const uint8_t* src_frame,
10199
int crop_x,
@@ -122,13 +120,8 @@ int ConvertFromI420(const VideoFrame& src_frame,
122120
// Compute PSNR for an I420 frame (all planes).
123121
// Returns the PSNR in decibel, to a maximum of kInfinitePSNR.
124122
double I420PSNR(const VideoFrame* ref_frame, const VideoFrame* test_frame);
125-
double I420PSNR(const VideoFrameBuffer& ref_buffer,
126-
const VideoFrameBuffer& test_buffer);
127-
128123
// Compute SSIM for an I420 frame (all planes).
129124
double I420SSIM(const VideoFrame* ref_frame, const VideoFrame* test_frame);
130-
double I420SSIM(const VideoFrameBuffer& ref_buffer,
131-
const VideoFrameBuffer& test_buffer);
132125

133126
} // namespace webrtc
134127

‎webrtc/common_video/libyuv/webrtc_libyuv.cc‎

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -290,55 +290,55 @@ int ConvertFromI420(const VideoFrame& src_frame,
290290
}
291291

292292
// Compute PSNR for an I420 frame (all planes)
293-
double I420PSNR(const VideoFrameBuffer& ref_buffer,
294-
const VideoFrameBuffer& test_buffer) {
295-
if ((ref_buffer.width() != test_buffer.width()) ||
296-
(ref_buffer.height() != test_buffer.height()))
293+
double I420PSNR(const VideoFrame* ref_frame, const VideoFrame* test_frame) {
294+
if (!ref_frame || !test_frame)
297295
return -1;
298-
else if (ref_buffer.width() < 0 || ref_buffer.height() < 0)
296+
else if ((ref_frame->width() != test_frame->width()) ||
297+
(ref_frame->height() != test_frame->height()))
298+
return -1;
299+
else if (ref_frame->width() < 0 || ref_frame->height() < 0)
299300
return -1;
300301

301-
double psnr = libyuv::I420Psnr(ref_buffer.DataY(), ref_buffer.StrideY(),
302-
ref_buffer.DataU(), ref_buffer.StrideU(),
303-
ref_buffer.DataV(), ref_buffer.StrideV(),
304-
test_buffer.DataY(), test_buffer.StrideY(),
305-
test_buffer.DataU(), test_buffer.StrideU(),
306-
test_buffer.DataV(), test_buffer.StrideV(),
307-
test_buffer.width(), test_buffer.height());
302+
double psnr = libyuv::I420Psnr(ref_frame->video_frame_buffer()->DataY(),
303+
ref_frame->video_frame_buffer()->StrideY(),
304+
ref_frame->video_frame_buffer()->DataU(),
305+
ref_frame->video_frame_buffer()->StrideU(),
306+
ref_frame->video_frame_buffer()->DataV(),
307+
ref_frame->video_frame_buffer()->StrideV(),
308+
test_frame->video_frame_buffer()->DataY(),
309+
test_frame->video_frame_buffer()->StrideY(),
310+
test_frame->video_frame_buffer()->DataU(),
311+
test_frame->video_frame_buffer()->StrideU(),
312+
test_frame->video_frame_buffer()->DataV(),
313+
test_frame->video_frame_buffer()->StrideV(),
314+
test_frame->width(), test_frame->height());
308315
// LibYuv sets the max psnr value to 128, we restrict it here.
309316
// In case of 0 mse in one frame, 128 can skew the results significantly.
310317
return (psnr > kPerfectPSNR) ? kPerfectPSNR : psnr;
311318
}
312319

313-
// Compute PSNR for an I420 frame (all planes)
314-
double I420PSNR(const VideoFrame* ref_frame, const VideoFrame* test_frame) {
320+
// Compute SSIM for an I420 frame (all planes)
321+
double I420SSIM(const VideoFrame* ref_frame, const VideoFrame* test_frame) {
315322
if (!ref_frame || !test_frame)
316323
return -1;
317-
return I420PSNR(*ref_frame->video_frame_buffer(),
318-
*test_frame->video_frame_buffer());
319-
}
320-
321-
// Compute SSIM for an I420 frame (all planes)
322-
double I420SSIM(const VideoFrameBuffer& ref_buffer,
323-
const VideoFrameBuffer& test_buffer) {
324-
if ((ref_buffer.width() != test_buffer.width()) ||
325-
(ref_buffer.height() != test_buffer.height()))
324+
else if ((ref_frame->width() != test_frame->width()) ||
325+
(ref_frame->height() != test_frame->height()))
326326
return -1;
327-
else if (ref_buffer.width() < 0 || ref_buffer.height() < 0)
327+
else if (ref_frame->width() < 0 || ref_frame->height() < 0)
328328
return -1;
329329

330-
return libyuv::I420Ssim(ref_buffer.DataY(), ref_buffer.StrideY(),
331-
ref_buffer.DataU(), ref_buffer.StrideU(),
332-
ref_buffer.DataV(), ref_buffer.StrideV(),
333-
test_buffer.DataY(), test_buffer.StrideY(),
334-
test_buffer.DataU(), test_buffer.StrideU(),
335-
test_buffer.DataV(), test_buffer.StrideV(),
336-
test_buffer.width(), test_buffer.height());
337-
}
338-
double I420SSIM(const VideoFrame* ref_frame, const VideoFrame* test_frame) {
339-
if (!ref_frame || !test_frame)
340-
return -1;
341-
return I420SSIM(*ref_frame->video_frame_buffer(),
342-
*test_frame->video_frame_buffer());
330+
return libyuv::I420Ssim(ref_frame->video_frame_buffer()->DataY(),
331+
ref_frame->video_frame_buffer()->StrideY(),
332+
ref_frame->video_frame_buffer()->DataU(),
333+
ref_frame->video_frame_buffer()->StrideU(),
334+
ref_frame->video_frame_buffer()->DataV(),
335+
ref_frame->video_frame_buffer()->StrideV(),
336+
test_frame->video_frame_buffer()->DataY(),
337+
test_frame->video_frame_buffer()->StrideY(),
338+
test_frame->video_frame_buffer()->DataU(),
339+
test_frame->video_frame_buffer()->StrideU(),
340+
test_frame->video_frame_buffer()->DataV(),
341+
test_frame->video_frame_buffer()->StrideV(),
342+
test_frame->width(), test_frame->height());
343343
}
344344
} // namespace webrtc

‎webrtc/test/fake_texture_frame.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ class FakeNativeHandleBuffer : public NativeHandleBuffer {
3838

3939
private:
4040
rtc::scoped_refptr<VideoFrameBuffer> NativeToI420Buffer() override {
41-
rtc::scoped_refptr<I420Buffer> buffer = I420Buffer::Create(width_, height_);
41+
rtc::scoped_refptr<VideoFrameBuffer> buffer(
42+
I420Buffer::Create(width_, height_));
4243
int half_height = (height_ + 1) / 2;
4344
int half_width = (width_ + 1) / 2;
4445
memset(buffer->MutableDataY(), 0, height_ * width_);

‎webrtc/test/frame_generator.cc‎

Lines changed: 32 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
#include "webrtc/base/checks.h"
1919
#include "webrtc/common_video/libyuv/include/webrtc_libyuv.h"
2020
#include "webrtc/system_wrappers/include/clock.h"
21-
#include "libyuv/convert.h"
2221

2322
namespace webrtc {
2423
namespace test {
@@ -27,41 +26,35 @@ namespace {
2726
class ChromaGenerator : public FrameGenerator {
2827
public:
2928
ChromaGenerator(size_t width, size_t height)
30-
: width_(width), height_(height), angle_(0.0) {
31-
RTC_CHECK(width_ > 0);
32-
RTC_CHECK(height_ > 0);
33-
half_width_ = (width_ + 1) / 2;
34-
y_size_ = width_ * height_;
35-
uv_size_ = half_width_ * ((height_ + 1) / 2);
29+
: angle_(0.0), width_(width), height_(height) {
30+
assert(width > 0);
31+
assert(height > 0);
3632
}
3733

3834
VideoFrame* NextFrame() override {
35+
frame_.CreateEmptyFrame(static_cast<int>(width_),
36+
static_cast<int>(height_),
37+
static_cast<int>(width_),
38+
static_cast<int>((width_ + 1) / 2),
39+
static_cast<int>((width_ + 1) / 2));
3940
angle_ += 30.0;
4041
uint8_t u = fabs(sin(angle_)) * 0xFF;
4142
uint8_t v = fabs(cos(angle_)) * 0xFF;
4243

43-
// Ensure stride == width.
44-
rtc::scoped_refptr<I420Buffer> buffer(I420Buffer::Create(
45-
static_cast<int>(width_), static_cast<int>(height_),
46-
static_cast<int>(width_), static_cast<int>(half_width_),
47-
static_cast<int>(half_width_)));
48-
49-
memset(buffer->MutableDataY(), 0x80, y_size_);
50-
memset(buffer->MutableDataU(), u, uv_size_);
51-
memset(buffer->MutableDataV(), v, uv_size_);
52-
53-
frame_.reset(new VideoFrame(buffer, 0, 0, webrtc::kVideoRotation_0));
54-
return frame_.get();
44+
memset(frame_.video_frame_buffer()->MutableDataY(), 0x80,
45+
frame_.allocated_size(kYPlane));
46+
memset(frame_.video_frame_buffer()->MutableDataU(), u,
47+
frame_.allocated_size(kUPlane));
48+
memset(frame_.video_frame_buffer()->MutableDataV(), v,
49+
frame_.allocated_size(kVPlane));
50+
return &frame_;
5551
}
5652

5753
private:
5854
double angle_;
5955
size_t width_;
6056
size_t height_;
61-
size_t half_width_;
62-
size_t y_size_;
63-
size_t uv_size_;
64-
std::unique_ptr<VideoFrame> frame_;
57+
VideoFrame frame_;
6558
};
6659

6760
class YuvFileGenerator : public FrameGenerator {
@@ -96,13 +89,15 @@ class YuvFileGenerator : public FrameGenerator {
9689
if (++current_display_count_ >= frame_display_count_)
9790
current_display_count_ = 0;
9891

99-
temp_frame_.reset(
100-
new VideoFrame(last_read_buffer_, 0, 0, webrtc::kVideoRotation_0));
101-
return temp_frame_.get();
92+
// If this is the last repeatition of this frame, it's OK to use the
93+
// original instance, otherwise use a copy.
94+
if (current_display_count_ == frame_display_count_)
95+
return &last_read_frame_;
96+
97+
temp_frame_copy_.CopyFrame(last_read_frame_);
98+
return &temp_frame_copy_;
10299
}
103100

104-
// TODO(nisse): Have a frame reader in one place. And read directly
105-
// into the planes of an I420Buffer, the extra copying below is silly.
106101
void ReadNextFrame() {
107102
size_t bytes_read =
108103
fread(frame_buffer_.get(), 1, frame_size_, files_[file_index_]);
@@ -115,21 +110,14 @@ class YuvFileGenerator : public FrameGenerator {
115110
assert(bytes_read >= frame_size_);
116111
}
117112

118-
size_t half_width = (width_ + 1) / 2;
119-
size_t size_y = width_ * height_;
120-
size_t size_uv = half_width * ((height_ + 1) / 2);
121-
last_read_buffer_ = I420Buffer::Create(
113+
last_read_frame_.CreateEmptyFrame(
122114
static_cast<int>(width_), static_cast<int>(height_),
123-
static_cast<int>(width_), static_cast<int>(half_width),
124-
static_cast<int>(half_width));
125-
libyuv::I420Copy(
126-
frame_buffer_.get(), static_cast<int>(width_),
127-
frame_buffer_.get() + size_y, static_cast<int>(half_width),
128-
frame_buffer_.get() + size_y + size_uv, static_cast<int>(half_width),
129-
last_read_buffer_->MutableDataY(), last_read_buffer_->StrideY(),
130-
last_read_buffer_->MutableDataU(), last_read_buffer_->StrideU(),
131-
last_read_buffer_->MutableDataV(), last_read_buffer_->StrideV(),
132-
static_cast<int>(width_), static_cast<int>(height_));
115+
static_cast<int>(width_), static_cast<int>((width_ + 1) / 2),
116+
static_cast<int>((width_ + 1) / 2));
117+
118+
ConvertToI420(kI420, frame_buffer_.get(), 0, 0, static_cast<int>(width_),
119+
static_cast<int>(height_), 0, kVideoRotation_0,
120+
&last_read_frame_);
133121
}
134122

135123
private:
@@ -141,8 +129,8 @@ class YuvFileGenerator : public FrameGenerator {
141129
const std::unique_ptr<uint8_t[]> frame_buffer_;
142130
const int frame_display_count_;
143131
int current_display_count_;
144-
rtc::scoped_refptr<I420Buffer> last_read_buffer_;
145-
std::unique_ptr<VideoFrame> temp_frame_;
132+
VideoFrame last_read_frame_;
133+
VideoFrame temp_frame_copy_;
146134
};
147135

148136
class ScrollingImageFrameGenerator : public FrameGenerator {

‎webrtc/test/testsupport/metrics/video_metrics.cc‎

Lines changed: 14 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
#include "webrtc/common_video/libyuv/include/webrtc_libyuv.h"
2020
#include "webrtc/video_frame.h"
21-
#include "libyuv/convert.h"
2221

2322
namespace webrtc {
2423
namespace test {
@@ -36,8 +35,8 @@ enum VideoMetricsType { kPSNR, kSSIM, kBoth };
3635

3736
// Calculates metrics for a frame and adds statistics to the result for it.
3837
void CalculateFrame(VideoMetricsType video_metrics_type,
39-
const VideoFrameBuffer& ref,
40-
const VideoFrameBuffer& test,
38+
const VideoFrame* ref,
39+
const VideoFrame* test,
4140
int frame_number,
4241
QualityMetricsResult* result) {
4342
FrameResult frame_result = {0, 0};
@@ -111,57 +110,37 @@ int CalculateMetrics(VideoMetricsType video_metrics_type,
111110

112111
// Read reference and test frames.
113112
const size_t frame_length = 3 * width * height >> 1;
114-
rtc::scoped_refptr<I420Buffer> ref_i420_buffer;
115-
rtc::scoped_refptr<I420Buffer> test_i420_buffer;
113+
VideoFrame ref_frame;
114+
VideoFrame test_frame;
116115
std::unique_ptr<uint8_t[]> ref_buffer(new uint8_t[frame_length]);
117116
std::unique_ptr<uint8_t[]> test_buffer(new uint8_t[frame_length]);
118117

119118
// Set decoded image parameters.
120119
int half_width = (width + 1) / 2;
121-
ref_i420_buffer =
122-
I420Buffer::Create(width, height, width, half_width, half_width);
123-
test_i420_buffer =
124-
I420Buffer::Create(width, height, width, half_width, half_width);
120+
ref_frame.CreateEmptyFrame(width, height, width, half_width, half_width);
121+
test_frame.CreateEmptyFrame(width, height, width, half_width, half_width);
125122

126-
// TODO(nisse): Have a frame reader in one place. And read directly
127-
// into the planes of an I420Buffer, the extra copying below is silly.
128123
size_t ref_bytes = fread(ref_buffer.get(), 1, frame_length, ref_fp);
129124
size_t test_bytes = fread(test_buffer.get(), 1, frame_length, test_fp);
130125
while (ref_bytes == frame_length && test_bytes == frame_length) {
131126
// Converting from buffer to plane representation.
132-
size_t size_y = width * height;
133-
size_t size_uv = half_width * ((height + 1) / 2);
134-
libyuv::I420Copy(
135-
ref_buffer.get(), width,
136-
ref_buffer.get() + size_y, half_width,
137-
ref_buffer.get() + size_y + size_uv, half_width,
138-
ref_i420_buffer->MutableDataY(), ref_i420_buffer->StrideY(),
139-
ref_i420_buffer->MutableDataU(), ref_i420_buffer->StrideU(),
140-
ref_i420_buffer->MutableDataV(), ref_i420_buffer->StrideV(),
141-
width, height);
142-
143-
libyuv::I420Copy(
144-
test_buffer.get(), width,
145-
test_buffer.get() + size_y, half_width,
146-
test_buffer.get() + size_y + size_uv, half_width,
147-
test_i420_buffer->MutableDataY(), test_i420_buffer->StrideY(),
148-
test_i420_buffer->MutableDataU(), test_i420_buffer->StrideU(),
149-
test_i420_buffer->MutableDataV(), test_i420_buffer->StrideV(),
150-
width, height);
151-
127+
ConvertToI420(kI420, ref_buffer.get(), 0, 0, width, height, 0,
128+
kVideoRotation_0, &ref_frame);
129+
ConvertToI420(kI420, test_buffer.get(), 0, 0, width, height, 0,
130+
kVideoRotation_0, &test_frame);
152131
switch (video_metrics_type) {
153132
case kPSNR:
154-
CalculateFrame(kPSNR, *ref_i420_buffer, *test_i420_buffer, frame_number,
133+
CalculateFrame(kPSNR, &ref_frame, &test_frame, frame_number,
155134
psnr_result);
156135
break;
157136
case kSSIM:
158-
CalculateFrame(kSSIM, *ref_i420_buffer, *test_i420_buffer, frame_number,
137+
CalculateFrame(kSSIM, &ref_frame, &test_frame, frame_number,
159138
ssim_result);
160139
break;
161140
case kBoth:
162-
CalculateFrame(kPSNR, *ref_i420_buffer, *test_i420_buffer, frame_number,
141+
CalculateFrame(kPSNR, &ref_frame, &test_frame, frame_number,
163142
psnr_result);
164-
CalculateFrame(kSSIM, *ref_i420_buffer, *test_i420_buffer, frame_number,
143+
CalculateFrame(kSSIM, &ref_frame, &test_frame, frame_number,
165144
ssim_result);
166145
break;
167146
}

0 commit comments

Comments
 (0)