Skip to content

Commit 1996e3f

Browse files
nisseCommit bot
nisse
authored and
Commit bot
committed
Reland of Update test code to use I420Buffer when writing pixel data. (patchset #1 id:1 of https://codereview.webrtc.org/2343083002/ )
Reason for revert: Will fix android build failure. Original issue's description: > 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 > > Committed: https://crrev.com/3011627142bccdd73fce9fec854abb1f6b02b5c1 > Cr-Commit-Position: refs/heads/master@{#14259} [email protected],[email protected],[email protected] # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true BUG=webrtc:5921 Review-Url: https://codereview.webrtc.org/2347863002 Cr-Commit-Position: refs/heads/master@{#14283}
1 parent afef413 commit 1996e3f

File tree

5 files changed

+123
-84
lines changed

5 files changed

+123
-84
lines changed

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

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

98+
// TODO(nisse): Deprecated, see
99+
// https://bugs.chromium.org/p/webrtc/issues/detail?id=5921.
98100
int ConvertToI420(VideoType src_video_type,
99101
const uint8_t* src_frame,
100102
int crop_x,
@@ -121,8 +123,13 @@ int ConvertFromI420(const VideoFrame& src_frame,
121123
// Compute PSNR for an I420 frame (all planes).
122124
// Returns the PSNR in decibel, to a maximum of kInfinitePSNR.
123125
double I420PSNR(const VideoFrame* ref_frame, const VideoFrame* test_frame);
126+
double I420PSNR(const VideoFrameBuffer& ref_buffer,
127+
const VideoFrameBuffer& test_buffer);
128+
124129
// Compute SSIM for an I420 frame (all planes).
125130
double I420SSIM(const VideoFrame* ref_frame, const VideoFrame* test_frame);
131+
double I420SSIM(const VideoFrameBuffer& ref_buffer,
132+
const VideoFrameBuffer& test_buffer);
126133

127134
// Helper class for directly converting and scaling NV12 to I420. The Y-plane
128135
// will be scaled directly to the I420 destination, which makes this faster

‎webrtc/common_video/libyuv/webrtc_libyuv.cc

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

292292
// Compute PSNR for an I420 frame (all planes)
293-
double I420PSNR(const VideoFrame* ref_frame, const VideoFrame* test_frame) {
294-
if (!ref_frame || !test_frame)
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()))
295297
return -1;
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)
298+
else if (ref_buffer.width() < 0 || ref_buffer.height() < 0)
300299
return -1;
301300

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());
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());
315308
// LibYuv sets the max psnr value to 128, we restrict it here.
316309
// In case of 0 mse in one frame, 128 can skew the results significantly.
317310
return (psnr > kPerfectPSNR) ? kPerfectPSNR : psnr;
318311
}
319312

320-
// Compute SSIM for an I420 frame (all planes)
321-
double I420SSIM(const VideoFrame* ref_frame, const VideoFrame* test_frame) {
313+
// Compute PSNR for an I420 frame (all planes)
314+
double I420PSNR(const VideoFrame* ref_frame, const VideoFrame* test_frame) {
322315
if (!ref_frame || !test_frame)
323316
return -1;
324-
else if ((ref_frame->width() != test_frame->width()) ||
325-
(ref_frame->height() != test_frame->height()))
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()))
326326
return -1;
327-
else if (ref_frame->width() < 0 || ref_frame->height() < 0)
327+
else if (ref_buffer.width() < 0 || ref_buffer.height() < 0)
328328
return -1;
329329

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());
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());
343343
}
344344

345345
void NV12ToI420Scaler::NV12ToI420Scale(

‎webrtc/test/fake_texture_frame.h

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

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

‎webrtc/test/frame_generator.cc

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
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"
2122

2223
namespace webrtc {
2324
namespace test {
@@ -27,34 +28,40 @@ class ChromaGenerator : public FrameGenerator {
2728
public:
2829
ChromaGenerator(size_t width, size_t height)
2930
: angle_(0.0), width_(width), height_(height) {
30-
assert(width > 0);
31-
assert(height > 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);
3236
}
3337

3438
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));
4039
angle_ += 30.0;
4140
uint8_t u = fabs(sin(angle_)) * 0xFF;
4241
uint8_t v = fabs(cos(angle_)) * 0xFF;
4342

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_;
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();
5155
}
5256

5357
private:
5458
double angle_;
5559
size_t width_;
5660
size_t height_;
57-
VideoFrame frame_;
61+
size_t half_width_;
62+
size_t y_size_;
63+
size_t uv_size_;
64+
std::unique_ptr<VideoFrame> frame_;
5865
};
5966

6067
class YuvFileGenerator : public FrameGenerator {
@@ -89,15 +96,13 @@ class YuvFileGenerator : public FrameGenerator {
8996
if (++current_display_count_ >= frame_display_count_)
9097
current_display_count_ = 0;
9198

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_;
99+
temp_frame_.reset(
100+
new VideoFrame(last_read_buffer_, 0, 0, webrtc::kVideoRotation_0));
101+
return temp_frame_.get();
99102
}
100103

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.
101106
void ReadNextFrame() {
102107
size_t bytes_read =
103108
fread(frame_buffer_.get(), 1, frame_size_, files_[file_index_]);
@@ -110,14 +115,21 @@ class YuvFileGenerator : public FrameGenerator {
110115
assert(bytes_read >= frame_size_);
111116
}
112117

113-
last_read_frame_.CreateEmptyFrame(
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(
114122
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_);
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_));
121133
}
122134

123135
private:
@@ -129,8 +141,8 @@ class YuvFileGenerator : public FrameGenerator {
129141
const std::unique_ptr<uint8_t[]> frame_buffer_;
130142
const int frame_display_count_;
131143
int current_display_count_;
132-
VideoFrame last_read_frame_;
133-
VideoFrame temp_frame_copy_;
144+
rtc::scoped_refptr<I420Buffer> last_read_buffer_;
145+
std::unique_ptr<VideoFrame> temp_frame_;
134146
};
135147

136148
class ScrollingImageFrameGenerator : public FrameGenerator {

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

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

1919
#include "webrtc/common_video/libyuv/include/webrtc_libyuv.h"
2020
#include "webrtc/video_frame.h"
21+
#include "libyuv/convert.h"
2122

2223
namespace webrtc {
2324
namespace test {
@@ -35,8 +36,8 @@ enum VideoMetricsType { kPSNR, kSSIM, kBoth };
3536

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

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

118119
// Set decoded image parameters.
119120
int half_width = (width + 1) / 2;
120-
ref_frame.CreateEmptyFrame(width, height, width, half_width, half_width);
121-
test_frame.CreateEmptyFrame(width, height, width, half_width, half_width);
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);
122125

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.
123128
size_t ref_bytes = fread(ref_buffer.get(), 1, frame_length, ref_fp);
124129
size_t test_bytes = fread(test_buffer.get(), 1, frame_length, test_fp);
125130
while (ref_bytes == frame_length && test_bytes == frame_length) {
126131
// Converting from buffer to plane representation.
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);
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+
131152
switch (video_metrics_type) {
132153
case kPSNR:
133-
CalculateFrame(kPSNR, &ref_frame, &test_frame, frame_number,
154+
CalculateFrame(kPSNR, *ref_i420_buffer, *test_i420_buffer, frame_number,
134155
psnr_result);
135156
break;
136157
case kSSIM:
137-
CalculateFrame(kSSIM, &ref_frame, &test_frame, frame_number,
158+
CalculateFrame(kSSIM, *ref_i420_buffer, *test_i420_buffer, frame_number,
138159
ssim_result);
139160
break;
140161
case kBoth:
141-
CalculateFrame(kPSNR, &ref_frame, &test_frame, frame_number,
162+
CalculateFrame(kPSNR, *ref_i420_buffer, *test_i420_buffer, frame_number,
142163
psnr_result);
143-
CalculateFrame(kSSIM, &ref_frame, &test_frame, frame_number,
164+
CalculateFrame(kSSIM, *ref_i420_buffer, *test_i420_buffer, frame_number,
144165
ssim_result);
145166
break;
146167
}

0 commit comments

Comments
 (0)