Commit e3e055a5 authored by Christopher Cameron's avatar Christopher Cameron Committed by Commit Bot

Fix grey bar when viewing local capture in Google Meet

Meet gives an option to request video quality of 360. Not all cameras
will natively return a height-360 size, so Chrome will go with the
next-biggest capture size, and letterbox-and-scale the video frame.

On the machine where I reproduced this, the media::VideoFrame had the
following attributes.
- visible_rect:0,60 640x360
- natural_size:640x360
- coded_size:640x480

This exposed the following bug in the combination of the functions
VideoResourceUpdater::CreateForSoftwarePlanes and
PaintCanvasVideoRenderer::Copy.
* The function VideoResourceUpdater::CreateForSoftwarePlanes creates a
  resource of size |coded_size|, and expects that
  PaintCanvasVideoRenderer::Copy will populate the |visible_rect|
  sub-rectangle of that resource.
* PaintCanvasVideoRenderer::Copy actually copies not to |visible_rect|,
  but rather to rectangle at (0,0) that has the size of |visible_rect|.

The consequence is that we end up viewing the resource through a
misaligned letterbox.

The fix is is to inline PaintCanvasVideoRenderer::Copy (which is only a
few lines long) in VideoResourceUpdater::CreateForSoftwarePlanes, and
specify the correct destination rectangle.

Also add some comments to clarify the function's behavior. Note that
this behavior was switched in crrev.com/545920.

Bug: 1090435
Change-Id: Icef81728defc0cf46b35180dada5d68f24e6b21d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2318313
Auto-Submit: ccameron <ccameron@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791480}
parent 2a5cfe96
......@@ -61,7 +61,9 @@ class MEDIA_EXPORT PaintCanvasVideoRenderer {
VideoTransformation video_transformation,
viz::RasterContextProvider* raster_context_provider);
// Paints |video_frame| scaled to its visible size on |canvas|.
// Paints |video_frame|, scaled to its |video_frame->visible_rect().size()|
// on |canvas|. Note that the origin of |video_frame->visible_rect()| is
// ignored -- the copy is done to the origin of |canvas|.
//
// If the format of |video_frame| is PIXEL_FORMAT_NATIVE_TEXTURE, |context_3d|
// and |context_support| must be provided.
......
......@@ -1015,11 +1015,19 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
SkBitmap sk_bitmap;
sk_bitmap.installPixels(info, software_resource->pixels(),
info.minRowBytes());
// This is software path, so |canvas| and |video_frame| are always
// backed by software.
cc::SkiaPaintCanvas canvas(sk_bitmap);
// This is software path, so canvas and video_frame are always backed
// by software.
video_renderer_->Copy(video_frame, &canvas, nullptr);
cc::PaintFlags flags;
flags.setBlendMode(SkBlendMode::kSrc);
flags.setFilterQuality(kLow_SkFilterQuality);
// Note that PaintCanvasVideoRenderer::Copy would copy to the origin,
// not |video_frame->visible_rect|, so call Paint instead.
// https://crbug.com/1090435
video_renderer_->Paint(video_frame, &canvas,
gfx::RectF(video_frame->visible_rect()), flags,
media::kNoTransformation, nullptr);
} else {
HardwarePlaneResource* hardware_resource = plane_resource->AsHardware();
size_t bytes_per_row = viz::ResourceSizes::CheckedWidthInBytes<size_t>(
......
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