Fix DCHECKs in V4l2VideoDevice.

- sizeimage DCHECK:
The video device can adjust the buffer size to be bigger
than we need as long as the data can fit. Theoretically
sizeimage should be bigger than PlaneAllocationSize. But
Exynos driver has a 256 byte padding for sizeimage. It is
rounded up when calculating coded_size. So sizeimage can
be smaller than PlaneAllocationSize. For example, say
format.fmt.pix_mp.width and height are 1920x1088.
format.fmt.pix_mp.plane_fmt[0].sizeimage is 2089216
(1920*1088+256). |sizeimage| is rounded up and |coded_size|
becomes 1920x1089. So format.fmt.pix_mp.plane_fmt[i].sizeimage
(2089216) is less than PlaneAllocationSize (1920*1090=2092800).

Also, we should not round down because the coded size is passed
VideoEncodeAccelerator::Client::RequireBitstreamBuffers.
The client will allocate buffers according to the coded size.

- bytesperline DCHECK:
bytesperline of different planes should be checked against
media::VideoFrame::row_bytes, not the width of coded size.

BUG=387701
TEST=Run Hangout in debug builds.

Review URL: https://codereview.chromium.org/406893002

Cr-Commit-Position: refs/heads/master@{#291374}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291374 0039d316-1c4b-4281-b951-d872f2087c98
parent 13ce0871
...@@ -214,6 +214,7 @@ void V4L2ImageProcessor::DestroyTask() { ...@@ -214,6 +214,7 @@ void V4L2ImageProcessor::DestroyTask() {
} }
bool V4L2ImageProcessor::CreateInputBuffers() { bool V4L2ImageProcessor::CreateInputBuffers() {
DVLOG(3) << __func__;
DCHECK(child_message_loop_proxy_->BelongsToCurrentThread()); DCHECK(child_message_loop_proxy_->BelongsToCurrentThread());
DCHECK(!input_streamon_); DCHECK(!input_streamon_);
...@@ -284,6 +285,7 @@ bool V4L2ImageProcessor::CreateInputBuffers() { ...@@ -284,6 +285,7 @@ bool V4L2ImageProcessor::CreateInputBuffers() {
} }
bool V4L2ImageProcessor::CreateOutputBuffers() { bool V4L2ImageProcessor::CreateOutputBuffers() {
DVLOG(3) << __func__;
DCHECK(child_message_loop_proxy_->BelongsToCurrentThread()); DCHECK(child_message_loop_proxy_->BelongsToCurrentThread());
DCHECK(!output_streamon_); DCHECK(!output_streamon_);
......
...@@ -106,6 +106,11 @@ gfx::Size V4L2Device::CodedSizeFromV4L2Format(struct v4l2_format format) { ...@@ -106,6 +106,11 @@ gfx::Size V4L2Device::CodedSizeFromV4L2Format(struct v4l2_format format) {
int horiz_bpp = int horiz_bpp =
media::VideoFrame::PlaneHorizontalBitsPerPixel(frame_format, 0); media::VideoFrame::PlaneHorizontalBitsPerPixel(frame_format, 0);
DVLOG(3) << __func__ << ": bytesperline=" << bytesperline
<< ", sizeimage=" << sizeimage
<< ", visible_size=" << visible_size.ToString() << ", frame_format="
<< media::VideoFrame::FormatToString(frame_format)
<< ", horiz_bpp=" << horiz_bpp;
if (sizeimage == 0 || bytesperline == 0 || horiz_bpp == 0 || if (sizeimage == 0 || bytesperline == 0 || horiz_bpp == 0 ||
(bytesperline * 8) % horiz_bpp != 0) { (bytesperline * 8) % horiz_bpp != 0) {
DLOG(ERROR) << "Invalid format provided"; DLOG(ERROR) << "Invalid format provided";
...@@ -118,6 +123,7 @@ gfx::Size V4L2Device::CodedSizeFromV4L2Format(struct v4l2_format format) { ...@@ -118,6 +123,7 @@ gfx::Size V4L2Device::CodedSizeFromV4L2Format(struct v4l2_format format) {
sizeimage = ((sizeimage + bytesperline - 1) / bytesperline) * bytesperline; sizeimage = ((sizeimage + bytesperline - 1) / bytesperline) * bytesperline;
coded_size.SetSize(bytesperline * 8 / horiz_bpp, sizeimage / bytesperline); coded_size.SetSize(bytesperline * 8 / horiz_bpp, sizeimage / bytesperline);
DVLOG(3) << "coded_size=" << coded_size.ToString();
// Sanity checks. Calculated coded size has to contain given visible size // Sanity checks. Calculated coded size has to contain given visible size
// and fulfill buffer byte size requirements for each plane. // and fulfill buffer byte size requirements for each plane.
...@@ -125,11 +131,9 @@ gfx::Size V4L2Device::CodedSizeFromV4L2Format(struct v4l2_format format) { ...@@ -125,11 +131,9 @@ gfx::Size V4L2Device::CodedSizeFromV4L2Format(struct v4l2_format format) {
if (V4L2_TYPE_IS_MULTIPLANAR(format.type)) { if (V4L2_TYPE_IS_MULTIPLANAR(format.type)) {
for (size_t i = 0; i < format.fmt.pix_mp.num_planes; ++i) { for (size_t i = 0; i < format.fmt.pix_mp.num_planes; ++i) {
DCHECK_LE( DCHECK_EQ(format.fmt.pix_mp.plane_fmt[i].bytesperline,
format.fmt.pix_mp.plane_fmt[i].sizeimage, base::checked_cast<__u32>(media::VideoFrame::RowBytes(
media::VideoFrame::PlaneAllocationSize(frame_format, i, coded_size)); i, frame_format, coded_size.width())));
DCHECK_LE(format.fmt.pix_mp.plane_fmt[i].bytesperline,
base::checked_cast<__u32>(coded_size.width()));
} }
} }
......
...@@ -860,6 +860,7 @@ bool V4L2VideoEncodeAccelerator::SetOutputFormat( ...@@ -860,6 +860,7 @@ bool V4L2VideoEncodeAccelerator::SetOutputFormat(
bool V4L2VideoEncodeAccelerator::NegotiateInputFormat( bool V4L2VideoEncodeAccelerator::NegotiateInputFormat(
media::VideoFrame::Format input_format) { media::VideoFrame::Format input_format) {
DVLOG(3) << "NegotiateInputFormat()";
DCHECK(child_message_loop_proxy_->BelongsToCurrentThread()); DCHECK(child_message_loop_proxy_->BelongsToCurrentThread());
DCHECK(!input_streamon_); DCHECK(!input_streamon_);
DCHECK(!output_streamon_); DCHECK(!output_streamon_);
......
...@@ -689,19 +689,20 @@ VideoFrame::~VideoFrame() { ...@@ -689,19 +689,20 @@ VideoFrame::~VideoFrame() {
base::ResetAndReturn(&no_longer_needed_cb_).Run(); base::ResetAndReturn(&no_longer_needed_cb_).Run();
} }
bool VideoFrame::IsValidPlane(size_t plane) const { // static
return (plane < NumPlanes(format_)); bool VideoFrame::IsValidPlane(size_t plane, VideoFrame::Format format) {
return (plane < NumPlanes(format));
} }
int VideoFrame::stride(size_t plane) const { int VideoFrame::stride(size_t plane) const {
DCHECK(IsValidPlane(plane)); DCHECK(IsValidPlane(plane, format_));
return strides_[plane]; return strides_[plane];
} }
int VideoFrame::row_bytes(size_t plane) const { // static
DCHECK(IsValidPlane(plane)); int VideoFrame::RowBytes(size_t plane, VideoFrame::Format format, int width) {
int width = coded_size_.width(); DCHECK(IsValidPlane(plane, format));
switch (format_) { switch (format) {
case VideoFrame::YV24: case VideoFrame::YV24:
switch (plane) { switch (plane) {
case kYPlane: case kYPlane:
...@@ -754,13 +755,17 @@ int VideoFrame::row_bytes(size_t plane) const { ...@@ -754,13 +755,17 @@ int VideoFrame::row_bytes(size_t plane) const {
case VideoFrame::NATIVE_TEXTURE: case VideoFrame::NATIVE_TEXTURE:
break; break;
} }
NOTREACHED() << "Unsupported video frame format/plane: " NOTREACHED() << "Unsupported video frame format/plane: " << format << "/"
<< format_ << "/" << plane; << plane;
return 0; return 0;
} }
int VideoFrame::row_bytes(size_t plane) const {
return RowBytes(plane, format_, coded_size_.width());
}
int VideoFrame::rows(size_t plane) const { int VideoFrame::rows(size_t plane) const {
DCHECK(IsValidPlane(plane)); DCHECK(IsValidPlane(plane, format_));
int height = coded_size_.height(); int height = coded_size_.height();
switch (format_) { switch (format_) {
case VideoFrame::YV24: case VideoFrame::YV24:
...@@ -822,7 +827,7 @@ int VideoFrame::rows(size_t plane) const { ...@@ -822,7 +827,7 @@ int VideoFrame::rows(size_t plane) const {
} }
uint8* VideoFrame::data(size_t plane) const { uint8* VideoFrame::data(size_t plane) const {
DCHECK(IsValidPlane(plane)); DCHECK(IsValidPlane(plane, format_));
return data_[plane]; return data_[plane];
} }
...@@ -854,7 +859,7 @@ int VideoFrame::dmabuf_fd(size_t plane) const { ...@@ -854,7 +859,7 @@ int VideoFrame::dmabuf_fd(size_t plane) const {
void VideoFrame::HashFrameForTesting(base::MD5Context* context) { void VideoFrame::HashFrameForTesting(base::MD5Context* context) {
for (int plane = 0; plane < kMaxPlanes; ++plane) { for (int plane = 0; plane < kMaxPlanes; ++plane) {
if (!IsValidPlane(plane)) if (!IsValidPlane(plane, format_))
break; break;
for (int row = 0; row < rows(plane); ++row) { for (int row = 0; row < rows(plane); ++row) {
base::MD5Update(context, base::StringPiece( base::MD5Update(context, base::StringPiece(
......
...@@ -219,6 +219,10 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> { ...@@ -219,6 +219,10 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
// Returns horizontal bits per pixel for given |plane| and |format|. // Returns horizontal bits per pixel for given |plane| and |format|.
static int PlaneHorizontalBitsPerPixel(Format format, size_t plane); static int PlaneHorizontalBitsPerPixel(Format format, size_t plane);
// Returns the number of bytes per row for the given plane, format, and width.
// The width may be aligned to format requirements.
static int RowBytes(size_t plane, Format format, int width);
Format format() const { return format_; } Format format() const { return format_; }
const gfx::Size& coded_size() const { return coded_size_; } const gfx::Size& coded_size() const { return coded_size_; }
...@@ -284,6 +288,11 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> { ...@@ -284,6 +288,11 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
private: private:
friend class base::RefCountedThreadSafe<VideoFrame>; friend class base::RefCountedThreadSafe<VideoFrame>;
// Returns true if |plane| is a valid plane number for the given format. This
// can be used to DCHECK() plane parameters.
static bool IsValidPlane(size_t plane, VideoFrame::Format format);
// Clients must use the static CreateFrame() method to create a new frame. // Clients must use the static CreateFrame() method to create a new frame.
VideoFrame(Format format, VideoFrame(Format format,
const gfx::Size& coded_size, const gfx::Size& coded_size,
...@@ -296,9 +305,6 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> { ...@@ -296,9 +305,6 @@ class MEDIA_EXPORT VideoFrame : public base::RefCountedThreadSafe<VideoFrame> {
void AllocateYUV(); void AllocateYUV();
// Used to DCHECK() plane parameters.
bool IsValidPlane(size_t plane) const;
// Frame format. // Frame format.
const Format format_; const Format format_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment