Commit 4f50a158 authored by Joe Downing's avatar Joe Downing Committed by Chromium LUCI CQ

Fixing a crash when using the H.264 encoder in Chrome Remote Desktop

TL;DR: calculate row size and offsets for |input_sample_| w/o using
the stride of the frame provided in ProcessInput().

I am working on getting our H.264 HW encoding solution running
again after ~3 year hiatus (we did some prototyping in 2017 but
haven't used it since).  This solution used to work but when I tried
it this week, I hit a crash in libyuv::I420ToNV12() which was called
from MediaFoundationVideoEncodeAccelerator::ProcessInput().

After some debugging, the crash appears to be due to the params
passed to libyuv::I420ToNV12().  The dst_* params get initialized
using the stride of the planes in the VideoFrame instead of the width
(in pixels) of the screen.

|input_sample_| is initialized using a size which does not take
stride into account so the libyuv conversion function eventually
runs off the edge of the input_buffer and AVs.

It looks like this change was introduced in:
https://chromium-review.googlesource.com/c/chromium/src/+/2124213

I'm not sure why other usages of this class have not hit this crash.
I suspect that either the usage in the Chrome browser use a
VideoFrame with no additional padding (stride - width == 0) or
there is something specific to how we are are using this class.

Change-Id: Id16415c93cc7f1180d1964e88be4b9db2a6ab36c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2572751
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834976}
parent 1e6a8460
......@@ -784,18 +784,18 @@ HRESULT MediaFoundationVideoEncodeAccelerator::ProcessInput(
{
MediaBufferScopedPointer scoped_buffer(input_buffer.Get());
DCHECK(scoped_buffer.get());
int dst_stride_y = frame->stride(VideoFrame::kYPlane);
uint8_t* dst_uv =
scoped_buffer.get() +
frame->stride(VideoFrame::kYPlane) * frame->rows(VideoFrame::kYPlane);
int dst_stride_uv = frame->stride(VideoFrame::kUPlane) * 2;
scoped_buffer.get() + frame->row_bytes(VideoFrame::kYPlane) *
frame->rows(VideoFrame::kYPlane);
libyuv::I420ToNV12(frame->visible_data(VideoFrame::kYPlane),
frame->stride(VideoFrame::kYPlane),
frame->visible_data(VideoFrame::kUPlane),
frame->stride(VideoFrame::kUPlane),
frame->visible_data(VideoFrame::kVPlane),
frame->stride(VideoFrame::kVPlane), scoped_buffer.get(),
dst_stride_y, dst_uv, dst_stride_uv,
frame->row_bytes(VideoFrame::kYPlane), dst_uv,
frame->row_bytes(VideoFrame::kUPlane) * 2,
input_visible_size_.width(),
input_visible_size_.height());
}
......
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