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
> > > R=magjed@webrtc.org, phoglund@webrtc.org
> > >
> > > Committed: https://crrev.com/280ad1514e44bf6717e5871526dd4632f759eb3d
> > > Cr-Commit-Position: refs/heads/master@{#14249}
> >
> > TBR=phoglund@webrtc.org,palmkvist@webrtc.org,magjed@webrtc.org
> > # 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}
>
> TBR=phoglund@webrtc.org,palmkvist@webrtc.org,magjed@webrtc.org
> # 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}
TBR=phoglund@webrtc.org,palmkvist@webrtc.org,magjed@webrtc.org
# 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}
diff --git a/webrtc/common_video/libyuv/include/webrtc_libyuv.h b/webrtc/common_video/libyuv/include/webrtc_libyuv.h
index ce1da3d..ec3720e 100644
--- a/webrtc/common_video/libyuv/include/webrtc_libyuv.h
+++ b/webrtc/common_video/libyuv/include/webrtc_libyuv.h
@@ -94,8 +94,6 @@
// - dst_frame : Reference to a destination frame.
// Return value: 0 if OK, < 0 otherwise.
-// TODO(nisse): Deprecated, see
-// https://bugs.chromium.org/p/webrtc/issues/detail?id=5921.
int ConvertToI420(VideoType src_video_type,
const uint8_t* src_frame,
int crop_x,
@@ -122,13 +120,8 @@
// Compute PSNR for an I420 frame (all planes).
// Returns the PSNR in decibel, to a maximum of kInfinitePSNR.
double I420PSNR(const VideoFrame* ref_frame, const VideoFrame* test_frame);
-double I420PSNR(const VideoFrameBuffer& ref_buffer,
- const VideoFrameBuffer& test_buffer);
-
// Compute SSIM for an I420 frame (all planes).
double I420SSIM(const VideoFrame* ref_frame, const VideoFrame* test_frame);
-double I420SSIM(const VideoFrameBuffer& ref_buffer,
- const VideoFrameBuffer& test_buffer);
} // namespace webrtc
diff --git a/webrtc/common_video/libyuv/webrtc_libyuv.cc b/webrtc/common_video/libyuv/webrtc_libyuv.cc
index 961b213..44577e9 100644
--- a/webrtc/common_video/libyuv/webrtc_libyuv.cc
+++ b/webrtc/common_video/libyuv/webrtc_libyuv.cc
@@ -290,55 +290,55 @@
}
// Compute PSNR for an I420 frame (all planes)
-double I420PSNR(const VideoFrameBuffer& ref_buffer,
- const VideoFrameBuffer& test_buffer) {
- if ((ref_buffer.width() != test_buffer.width()) ||
- (ref_buffer.height() != test_buffer.height()))
+double I420PSNR(const VideoFrame* ref_frame, const VideoFrame* test_frame) {
+ if (!ref_frame || !test_frame)
return -1;
- else if (ref_buffer.width() < 0 || ref_buffer.height() < 0)
+ else if ((ref_frame->width() != test_frame->width()) ||
+ (ref_frame->height() != test_frame->height()))
+ return -1;
+ else if (ref_frame->width() < 0 || ref_frame->height() < 0)
return -1;
- double psnr = libyuv::I420Psnr(ref_buffer.DataY(), ref_buffer.StrideY(),
- ref_buffer.DataU(), ref_buffer.StrideU(),
- ref_buffer.DataV(), ref_buffer.StrideV(),
- test_buffer.DataY(), test_buffer.StrideY(),
- test_buffer.DataU(), test_buffer.StrideU(),
- test_buffer.DataV(), test_buffer.StrideV(),
- test_buffer.width(), test_buffer.height());
+ double psnr = libyuv::I420Psnr(ref_frame->video_frame_buffer()->DataY(),
+ ref_frame->video_frame_buffer()->StrideY(),
+ ref_frame->video_frame_buffer()->DataU(),
+ ref_frame->video_frame_buffer()->StrideU(),
+ ref_frame->video_frame_buffer()->DataV(),
+ ref_frame->video_frame_buffer()->StrideV(),
+ test_frame->video_frame_buffer()->DataY(),
+ test_frame->video_frame_buffer()->StrideY(),
+ test_frame->video_frame_buffer()->DataU(),
+ test_frame->video_frame_buffer()->StrideU(),
+ test_frame->video_frame_buffer()->DataV(),
+ test_frame->video_frame_buffer()->StrideV(),
+ test_frame->width(), test_frame->height());
// LibYuv sets the max psnr value to 128, we restrict it here.
// In case of 0 mse in one frame, 128 can skew the results significantly.
return (psnr > kPerfectPSNR) ? kPerfectPSNR : psnr;
}
-// Compute PSNR for an I420 frame (all planes)
-double I420PSNR(const VideoFrame* ref_frame, const VideoFrame* test_frame) {
- if (!ref_frame || !test_frame)
- return -1;
- return I420PSNR(*ref_frame->video_frame_buffer(),
- *test_frame->video_frame_buffer());
-}
-
// Compute SSIM for an I420 frame (all planes)
-double I420SSIM(const VideoFrameBuffer& ref_buffer,
- const VideoFrameBuffer& test_buffer) {
- if ((ref_buffer.width() != test_buffer.width()) ||
- (ref_buffer.height() != test_buffer.height()))
- return -1;
- else if (ref_buffer.width() < 0 || ref_buffer.height() < 0)
- return -1;
-
- return libyuv::I420Ssim(ref_buffer.DataY(), ref_buffer.StrideY(),
- ref_buffer.DataU(), ref_buffer.StrideU(),
- ref_buffer.DataV(), ref_buffer.StrideV(),
- test_buffer.DataY(), test_buffer.StrideY(),
- test_buffer.DataU(), test_buffer.StrideU(),
- test_buffer.DataV(), test_buffer.StrideV(),
- test_buffer.width(), test_buffer.height());
-}
double I420SSIM(const VideoFrame* ref_frame, const VideoFrame* test_frame) {
if (!ref_frame || !test_frame)
return -1;
- return I420SSIM(*ref_frame->video_frame_buffer(),
- *test_frame->video_frame_buffer());
+ else if ((ref_frame->width() != test_frame->width()) ||
+ (ref_frame->height() != test_frame->height()))
+ return -1;
+ else if (ref_frame->width() < 0 || ref_frame->height() < 0)
+ return -1;
+
+ return libyuv::I420Ssim(ref_frame->video_frame_buffer()->DataY(),
+ ref_frame->video_frame_buffer()->StrideY(),
+ ref_frame->video_frame_buffer()->DataU(),
+ ref_frame->video_frame_buffer()->StrideU(),
+ ref_frame->video_frame_buffer()->DataV(),
+ ref_frame->video_frame_buffer()->StrideV(),
+ test_frame->video_frame_buffer()->DataY(),
+ test_frame->video_frame_buffer()->StrideY(),
+ test_frame->video_frame_buffer()->DataU(),
+ test_frame->video_frame_buffer()->StrideU(),
+ test_frame->video_frame_buffer()->DataV(),
+ test_frame->video_frame_buffer()->StrideV(),
+ test_frame->width(), test_frame->height());
}
} // namespace webrtc
diff --git a/webrtc/test/fake_texture_frame.h b/webrtc/test/fake_texture_frame.h
index 5e7571d..23b2954 100644
--- a/webrtc/test/fake_texture_frame.h
+++ b/webrtc/test/fake_texture_frame.h
@@ -38,7 +38,8 @@
private:
rtc::scoped_refptr<VideoFrameBuffer> NativeToI420Buffer() override {
- rtc::scoped_refptr<I420Buffer> buffer = I420Buffer::Create(width_, height_);
+ rtc::scoped_refptr<VideoFrameBuffer> buffer(
+ I420Buffer::Create(width_, height_));
int half_height = (height_ + 1) / 2;
int half_width = (width_ + 1) / 2;
memset(buffer->MutableDataY(), 0, height_ * width_);
diff --git a/webrtc/test/frame_generator.cc b/webrtc/test/frame_generator.cc
index 4bfb9ea..ed7e95a 100644
--- a/webrtc/test/frame_generator.cc
+++ b/webrtc/test/frame_generator.cc
@@ -18,7 +18,6 @@
#include "webrtc/base/checks.h"
#include "webrtc/common_video/libyuv/include/webrtc_libyuv.h"
#include "webrtc/system_wrappers/include/clock.h"
-#include "libyuv/convert.h"
namespace webrtc {
namespace test {
@@ -27,41 +26,35 @@
class ChromaGenerator : public FrameGenerator {
public:
ChromaGenerator(size_t width, size_t height)
- : width_(width), height_(height), angle_(0.0) {
- RTC_CHECK(width_ > 0);
- RTC_CHECK(height_ > 0);
- half_width_ = (width_ + 1) / 2;
- y_size_ = width_ * height_;
- uv_size_ = half_width_ * ((height_ + 1) / 2);
+ : angle_(0.0), width_(width), height_(height) {
+ assert(width > 0);
+ assert(height > 0);
}
VideoFrame* NextFrame() override {
+ frame_.CreateEmptyFrame(static_cast<int>(width_),
+ static_cast<int>(height_),
+ static_cast<int>(width_),
+ static_cast<int>((width_ + 1) / 2),
+ static_cast<int>((width_ + 1) / 2));
angle_ += 30.0;
uint8_t u = fabs(sin(angle_)) * 0xFF;
uint8_t v = fabs(cos(angle_)) * 0xFF;
- // Ensure stride == width.
- rtc::scoped_refptr<I420Buffer> buffer(I420Buffer::Create(
- static_cast<int>(width_), static_cast<int>(height_),
- static_cast<int>(width_), static_cast<int>(half_width_),
- static_cast<int>(half_width_)));
-
- memset(buffer->MutableDataY(), 0x80, y_size_);
- memset(buffer->MutableDataU(), u, uv_size_);
- memset(buffer->MutableDataV(), v, uv_size_);
-
- frame_.reset(new VideoFrame(buffer, 0, 0, webrtc::kVideoRotation_0));
- return frame_.get();
+ memset(frame_.video_frame_buffer()->MutableDataY(), 0x80,
+ frame_.allocated_size(kYPlane));
+ memset(frame_.video_frame_buffer()->MutableDataU(), u,
+ frame_.allocated_size(kUPlane));
+ memset(frame_.video_frame_buffer()->MutableDataV(), v,
+ frame_.allocated_size(kVPlane));
+ return &frame_;
}
private:
double angle_;
size_t width_;
size_t height_;
- size_t half_width_;
- size_t y_size_;
- size_t uv_size_;
- std::unique_ptr<VideoFrame> frame_;
+ VideoFrame frame_;
};
class YuvFileGenerator : public FrameGenerator {
@@ -96,13 +89,15 @@
if (++current_display_count_ >= frame_display_count_)
current_display_count_ = 0;
- temp_frame_.reset(
- new VideoFrame(last_read_buffer_, 0, 0, webrtc::kVideoRotation_0));
- return temp_frame_.get();
+ // If this is the last repeatition of this frame, it's OK to use the
+ // original instance, otherwise use a copy.
+ if (current_display_count_ == frame_display_count_)
+ return &last_read_frame_;
+
+ temp_frame_copy_.CopyFrame(last_read_frame_);
+ return &temp_frame_copy_;
}
- // TODO(nisse): Have a frame reader in one place. And read directly
- // into the planes of an I420Buffer, the extra copying below is silly.
void ReadNextFrame() {
size_t bytes_read =
fread(frame_buffer_.get(), 1, frame_size_, files_[file_index_]);
@@ -115,21 +110,14 @@
assert(bytes_read >= frame_size_);
}
- size_t half_width = (width_ + 1) / 2;
- size_t size_y = width_ * height_;
- size_t size_uv = half_width * ((height_ + 1) / 2);
- last_read_buffer_ = I420Buffer::Create(
+ last_read_frame_.CreateEmptyFrame(
static_cast<int>(width_), static_cast<int>(height_),
- static_cast<int>(width_), static_cast<int>(half_width),
- static_cast<int>(half_width));
- libyuv::I420Copy(
- frame_buffer_.get(), static_cast<int>(width_),
- frame_buffer_.get() + size_y, static_cast<int>(half_width),
- frame_buffer_.get() + size_y + size_uv, static_cast<int>(half_width),
- last_read_buffer_->MutableDataY(), last_read_buffer_->StrideY(),
- last_read_buffer_->MutableDataU(), last_read_buffer_->StrideU(),
- last_read_buffer_->MutableDataV(), last_read_buffer_->StrideV(),
- static_cast<int>(width_), static_cast<int>(height_));
+ static_cast<int>(width_), static_cast<int>((width_ + 1) / 2),
+ static_cast<int>((width_ + 1) / 2));
+
+ ConvertToI420(kI420, frame_buffer_.get(), 0, 0, static_cast<int>(width_),
+ static_cast<int>(height_), 0, kVideoRotation_0,
+ &last_read_frame_);
}
private:
@@ -141,8 +129,8 @@
const std::unique_ptr<uint8_t[]> frame_buffer_;
const int frame_display_count_;
int current_display_count_;
- rtc::scoped_refptr<I420Buffer> last_read_buffer_;
- std::unique_ptr<VideoFrame> temp_frame_;
+ VideoFrame last_read_frame_;
+ VideoFrame temp_frame_copy_;
};
class ScrollingImageFrameGenerator : public FrameGenerator {
diff --git a/webrtc/test/testsupport/metrics/video_metrics.cc b/webrtc/test/testsupport/metrics/video_metrics.cc
index 75bbc61..ee9aa34 100644
--- a/webrtc/test/testsupport/metrics/video_metrics.cc
+++ b/webrtc/test/testsupport/metrics/video_metrics.cc
@@ -18,7 +18,6 @@
#include "webrtc/common_video/libyuv/include/webrtc_libyuv.h"
#include "webrtc/video_frame.h"
-#include "libyuv/convert.h"
namespace webrtc {
namespace test {
@@ -36,8 +35,8 @@
// Calculates metrics for a frame and adds statistics to the result for it.
void CalculateFrame(VideoMetricsType video_metrics_type,
- const VideoFrameBuffer& ref,
- const VideoFrameBuffer& test,
+ const VideoFrame* ref,
+ const VideoFrame* test,
int frame_number,
QualityMetricsResult* result) {
FrameResult frame_result = {0, 0};
@@ -111,57 +110,37 @@
// Read reference and test frames.
const size_t frame_length = 3 * width * height >> 1;
- rtc::scoped_refptr<I420Buffer> ref_i420_buffer;
- rtc::scoped_refptr<I420Buffer> test_i420_buffer;
+ VideoFrame ref_frame;
+ VideoFrame test_frame;
std::unique_ptr<uint8_t[]> ref_buffer(new uint8_t[frame_length]);
std::unique_ptr<uint8_t[]> test_buffer(new uint8_t[frame_length]);
// Set decoded image parameters.
int half_width = (width + 1) / 2;
- ref_i420_buffer =
- I420Buffer::Create(width, height, width, half_width, half_width);
- test_i420_buffer =
- I420Buffer::Create(width, height, width, half_width, half_width);
+ ref_frame.CreateEmptyFrame(width, height, width, half_width, half_width);
+ test_frame.CreateEmptyFrame(width, height, width, half_width, half_width);
- // TODO(nisse): Have a frame reader in one place. And read directly
- // into the planes of an I420Buffer, the extra copying below is silly.
size_t ref_bytes = fread(ref_buffer.get(), 1, frame_length, ref_fp);
size_t test_bytes = fread(test_buffer.get(), 1, frame_length, test_fp);
while (ref_bytes == frame_length && test_bytes == frame_length) {
// Converting from buffer to plane representation.
- size_t size_y = width * height;
- size_t size_uv = half_width * ((height + 1) / 2);
- libyuv::I420Copy(
- ref_buffer.get(), width,
- ref_buffer.get() + size_y, half_width,
- ref_buffer.get() + size_y + size_uv, half_width,
- ref_i420_buffer->MutableDataY(), ref_i420_buffer->StrideY(),
- ref_i420_buffer->MutableDataU(), ref_i420_buffer->StrideU(),
- ref_i420_buffer->MutableDataV(), ref_i420_buffer->StrideV(),
- width, height);
-
- libyuv::I420Copy(
- test_buffer.get(), width,
- test_buffer.get() + size_y, half_width,
- test_buffer.get() + size_y + size_uv, half_width,
- test_i420_buffer->MutableDataY(), test_i420_buffer->StrideY(),
- test_i420_buffer->MutableDataU(), test_i420_buffer->StrideU(),
- test_i420_buffer->MutableDataV(), test_i420_buffer->StrideV(),
- width, height);
-
+ ConvertToI420(kI420, ref_buffer.get(), 0, 0, width, height, 0,
+ kVideoRotation_0, &ref_frame);
+ ConvertToI420(kI420, test_buffer.get(), 0, 0, width, height, 0,
+ kVideoRotation_0, &test_frame);
switch (video_metrics_type) {
case kPSNR:
- CalculateFrame(kPSNR, *ref_i420_buffer, *test_i420_buffer, frame_number,
+ CalculateFrame(kPSNR, &ref_frame, &test_frame, frame_number,
psnr_result);
break;
case kSSIM:
- CalculateFrame(kSSIM, *ref_i420_buffer, *test_i420_buffer, frame_number,
+ CalculateFrame(kSSIM, &ref_frame, &test_frame, frame_number,
ssim_result);
break;
case kBoth:
- CalculateFrame(kPSNR, *ref_i420_buffer, *test_i420_buffer, frame_number,
+ CalculateFrame(kPSNR, &ref_frame, &test_frame, frame_number,
psnr_result);
- CalculateFrame(kSSIM, *ref_i420_buffer, *test_i420_buffer, frame_number,
+ CalculateFrame(kSSIM, &ref_frame, &test_frame, frame_number,
ssim_result);
break;
}
