Commit ff4f5fb6 authored by Andres Calderon Jaramillo's avatar Andres Calderon Jaramillo Committed by Commit Bot

Handle visible_rect in V4L2/VAAPI VDAs when origin != (0, 0).

This CL modifies the V4L2 and VAAPI video decode accelerators to handle
the case where the visible rectangle is not (0, 0). This can happen with
some H.264 videos.

The solution is to make sure that when we create the GL image for a
frame, the size should include the visible rectangle plus the
non-visible area to the left and on the top of that rectangle. This way,
the compositor can calculate the UV coordinates correctly so that we can
sample only the visible rectangle.

V4L2VideoDecodeAccelerator is unchanged. The reason is that the
assumption that the visible rectangle starts at (0, 0) will be harder to
remove there. We currently have a check that deals with that case [1].

CL:2041712 + (CL:2122484 + this) make it possible to handle exotic
visible rectangles in most Chrome OS devices (the ones that use
V4L2VideoDecodeAccelerator still need work). [2] contains before/after
tests for this CL for a variety of devices (where before is 822a983f minus
CL:2122484).

[1] https://cs.chromium.org/chromium/src/media/gpu/v4l2/v4l2_video_decode_accelerator.cc?l=2272-2277&rcl=10aaea0473eb652966b359c2b751974b97c5ab13
[2] https://drive.google.com/open?id=1wtX7jDkzvIxCMmsQlIV7zjwaWMrmikUy

Bug: 1062002
Test: H.264 with unusual rect in kevin, kukui, eve, hana, nyan_kitty.
Change-Id: Iac712fbf1b2cea3556b39fc38f5639c91c73b746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2128707
Commit-Queue: Andres Calderon Jaramillo <andrescj@chromium.org>
Reviewed-by: default avatarDan Sanders <sandersd@chromium.org>
Reviewed-by: default avatarAlexandre Courbot <acourbot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755273}
parent a42ef137
......@@ -382,6 +382,10 @@ gfx::Size ScaleSizeToEncompassTarget(const gfx::Size& size,
return ScaleSizeToTarget(size, target, false);
}
gfx::Size GetRectSizeFromOrigin(const gfx::Rect& rect) {
return gfx::Size(rect.right(), rect.bottom());
}
gfx::Size PadToMatchAspectRatio(const gfx::Size& size,
const gfx::Size& target) {
if (target.IsEmpty())
......
......@@ -114,6 +114,17 @@ MEDIA_EXPORT gfx::Size ScaleSizeToFitWithinTarget(const gfx::Size& size,
MEDIA_EXPORT gfx::Size ScaleSizeToEncompassTarget(const gfx::Size& size,
const gfx::Size& target);
// Returns the size of a rectangle whose upper left corner is at the origin (0,
// 0) and whose bottom right corner is the same as that of |rect|. This is
// useful to get the size of a buffer that contains the visible rectangle plus
// the non-visible area above and to the left of the visible rectangle.
//
// An example to illustrate: suppose the visible rectangle of a decoded frame is
// 10,10,100,100. The size of this rectangle is 90x90. However, we need to
// create a texture of size 100x100 because the client will want to sample from
// the texture starting with uv coordinates corresponding to 10,10.
MEDIA_EXPORT gfx::Size GetRectSizeFromOrigin(const gfx::Rect& rect);
// Returns |size| with only one of its dimensions increased such that the result
// matches the aspect ratio of |target|. This is different from
// ScaleSizeToEncompassTarget() in two ways: 1) The goal is to match the aspect
......
......@@ -535,15 +535,15 @@ void VdaVideoDecoder::ProvidePictureBuffersWithVisibleRect(
// that the client uses should be only 640x360. Therefore, we pass
// |visible_rect|.size() here as the requested size of the picture buffers.
//
// TODO(andrescj): this is not correct in the case that the visible rectangle
// does not start at (0, 0). This can happen for some exotic H.264 videos. For
// example, if the coded size is 640x368 and the visible rectangle is
// 2,2,640x360, the size of the picture buffers we pass here should be
// 642x362. That's because the compositor is responsible for calculating the
// UV coordinates in such a way that the non-visible area to the left and on
// the top of the visible rectangle are not displayed.
// Note that we use GetRectSizeFromOrigin() to handle the unusual case in
// which the visible rectangle does not start at (0, 0). For example, for an
// H.264 video with a visible rectangle of 2,2,635x360, the coded size is
// 640x360, but the GL texture that the client uses should be 637x362. This is
// because we want the texture to include the non-visible area to the left and
// on the top of the visible rectangle so that the compositor can calculate
// the UV coordinates to omit the non-visible area.
ProvidePictureBuffers(requested_num_of_buffers, format, textures_per_buffer,
visible_rect.size(), texture_target);
GetRectSizeFromOrigin(visible_rect), texture_target);
}
void VdaVideoDecoder::ProvidePictureBuffersAsync(uint32_t count,
......
......@@ -36,6 +36,7 @@
#include "media/base/scopedfd_helper.h"
#include "media/base/unaligned_shared_memory.h"
#include "media/base/video_types.h"
#include "media/base/video_util.h"
#include "media/gpu/chromeos/fourcc.h"
#include "media/gpu/chromeos/platform_video_frame_utils.h"
#include "media/gpu/macros.h"
......@@ -590,7 +591,7 @@ bool V4L2SliceVideoDecodeAccelerator::CreateImageProcessor() {
image_processor_ = v4l2_vda_helpers::CreateImageProcessor(
*output_format_fourcc_, *gl_image_format_fourcc_, coded_size_,
gl_image_size_, decoder_->GetVisibleRect().size(),
gl_image_size_, GetRectSizeFromOrigin(decoder_->GetVisibleRect()),
output_buffer_map_.size(), image_processor_device_,
image_processor_output_mode,
// Unretained(this) is safe for ErrorCB because |decoder_thread_| is owned
......@@ -692,7 +693,7 @@ bool V4L2SliceVideoDecodeAccelerator::CreateOutputBuffers() {
if (image_processor_device_) {
// Try to get an image size as close as possible to the final one (i.e.
// coded_size_ may include padding required by the decoder).
gl_image_size_ = decoder_->GetVisibleRect().size();
gl_image_size_ = GetRectSizeFromOrigin(decoder_->GetVisibleRect());
size_t planes_count;
if (!V4L2ImageProcessorBackend::TryOutputFormat(
output_format_fourcc_->ToV4L2PixFmt(),
......@@ -1273,7 +1274,7 @@ void V4L2SliceVideoDecodeAccelerator::AssignPictureBuffersTask(
const gfx::Size pic_size_received_from_client = buffers[0].size();
const gfx::Size pic_size_expected_from_client =
output_mode_ == Config::OutputMode::ALLOCATE
? decoder_->GetVisibleRect().size()
? GetRectSizeFromOrigin(decoder_->GetVisibleRect())
: coded_size_;
if (output_mode_ == Config::OutputMode::ALLOCATE &&
pic_size_expected_from_client != pic_size_received_from_client) {
......@@ -1609,7 +1610,8 @@ void V4L2SliceVideoDecodeAccelerator::ImportBufferForPictureTask(
base::BindOnce(&V4L2SliceVideoDecodeAccelerator::CreateGLImageFor,
weak_this_, device_, index, picture_buffer_id,
std::move(handle), iter->client_texture_id,
iter->texture_id, decoder_->GetVisibleRect().size(),
iter->texture_id,
GetRectSizeFromOrigin(decoder_->GetVisibleRect()),
*gl_image_format_fourcc_));
}
......@@ -2273,7 +2275,8 @@ void V4L2SliceVideoDecodeAccelerator::FrameProcessed(
ip_output_record.picture_id,
CreateGpuMemoryBufferHandle(frame.get()).native_pixmap_handle,
ip_output_record.client_texture_id, ip_output_record.texture_id,
decoder_->GetVisibleRect().size(), *gl_image_format_fourcc_));
GetRectSizeFromOrigin(decoder_->GetVisibleRect()),
*gl_image_format_fourcc_));
}
DCHECK(!surfaces_at_ip_.empty());
......
......@@ -28,6 +28,7 @@
#include "media/base/bind_to_current_loop.h"
#include "media/base/format_utils.h"
#include "media/base/unaligned_shared_memory.h"
#include "media/base/video_util.h"
#include "media/gpu/accelerated_video_decoder.h"
#include "media/gpu/h264_decoder.h"
#include "media/gpu/macros.h"
......@@ -689,7 +690,7 @@ void VaapiVideoDecodeAccelerator::AssignPictureBuffers(
// Note that the |size_to_bind| is not relevant in IMPORT mode.
const gfx::Size size_to_bind =
(output_mode_ == Config::OutputMode::ALLOCATE)
? requested_visible_rect_.size()
? GetRectSizeFromOrigin(requested_visible_rect_)
: gfx::Size();
std::unique_ptr<VaapiPicture> picture = vaapi_picture_factory_->Create(
......
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