Skip to content

GitLab

T
tangled
Commit 1364e616 authored by Sunny Sachanandani's avatar Sunny Sachanandani Committed by Commit Bot
Browse files
Options

Reland "media: Use GL_UNPACK_ROW_LENGTH for software planes stride adaptation" 

This is a reland of b71115a3

Relanding after triaging Gold failures on gpu.fyi bots in previous
attempt.

Original change's description:
> media: Use GL_UNPACK_ROW_LENGTH for software planes stride adaptation
>
> Avoid a CPU side copy to account for mismatched strides between the
> decoded video frame and GPU texture created for uploading the frame.
> GL_UNPACK_ROW_LENGTH allows specifying the stride for glTexSubImage2D,
> and is part of the GL_EXT_unpack_subimage extension implemented by the
> command buffer on the client side.
>
> Also includes a pixel test with a VP8 video with a GL incompatible
> stride that doesn't match the video's coded size (992 vs 962).
>
> Bug: 1077211
> Change-Id: I62753234bde3b92e64089c92a59e65ae2bc1c84c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2386671
> Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
> Reviewed-by: Zhenyao Mo <zmo@chromium.org>
> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#803696}

TBR=dalecurtis@chromium.org,zmo@chromium.org

Bug: 1077211, 1124215
Change-Id: I43a97e72cdd0ba633d4b374a2319fbc7dc7252f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2390958Reviewed-by: default avatarSunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803960}
parent dc76f117
Hide whitespace changes
Inline Side-by-side
Showing with 58 additions and 17 deletions
content/test/BUILD.gn
View file @ 1364e616
......@@ -668,6 +668,7 @@ group("telemetry_gpu_integration_test_support") {
"//media/test/data/four-colors-rot-90.mp4",
"//media/test/data/four-colors-rot-180.mp4",
"//media/test/data/four-colors-rot-270.mp4",
"//media/test/data/four-colors-vp8-incompatible-stride.webm",
"//media/test/data/four-colors-vp9.webm",
"//media/test/data/four-colors-vp9-i420a.webm",
......
content/test/data/gpu/pixel_video_vp8_incompatible_stride.html 0 → 100644
View file @ 1364e616
<!DOCTYPE HTML>
<html>
<head>
<title>VP8 Incompatible Stride Video test</title>
<style type="text/css">
.nomargin {
margin: 0px auto;
}
</style>
<script src="pixel_video_test.js"></script>
</head>
<body onload="main()">
<div id="container" style="position:absolute; top:0px; left:0px">
<video class="nomargin" id="video" width="240" height="135">
<source src="/media/test/data/four-colors-vp8-incompatible-stride.webm" type="video/webm">
</video>
</div>
</body>
</html>
content/test/gpu/gpu_tests/pixel_test_pages.py
View file @ 1364e616
......@@ -209,6 +209,10 @@ class PixelTestPages(object):
max_different_pixels=30500,
pixel_delta_threshold=15,
edge_threshold=70)),
PixelTestPage('pixel_video_vp8_incompatible_stride.html',
base_name + '_Video_VP8_Incompatible_Stride',
test_rect=[0, 0, 240, 135],
matching_algorithm=VERY_PERMISSIVE_SOBEL_ALGO),
PixelTestPage('pixel_video_vp9.html',
base_name + '_Video_VP9',
test_rect=[0, 0, 240, 135],
......
content/test/gpu/gpu_tests/test_expectations/pixel_expectations.txt
View file @ 1364e616
......@@ -77,6 +77,7 @@ crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_MP4_FourColors_Rot_18
crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_MP4_FourColors_Rot_270 [ Skip ]
crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_MP4_FourColors_Rot_90 [ Skip ]
crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_MP4 [ Skip ]
crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_VP8_Incompatible_Stride [ Skip ]
crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_VP9 [ Skip ]
crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_VP9_DXVA [ Skip ]
......@@ -356,6 +357,7 @@ crbug.com/1021566 [ use-skia-dawn skia-renderer ] Pixel_Video_MP4_FourColors_Asp
crbug.com/1021566 [ use-skia-dawn skia-renderer ] Pixel_Video_MP4_FourColors_Rot_180 [ Skip ]
crbug.com/1021566 [ use-skia-dawn skia-renderer ] Pixel_Video_MP4_FourColors_Rot_270 [ Skip ]
crbug.com/1021566 [ use-skia-dawn skia-renderer ] Pixel_Video_MP4_FourColors_Rot_90 [ Skip ]
crbug.com/1021566 [ use-skia-dawn ] Pixel_Video_VP8_Incompatible_Stride [ Skip ]
crbug.com/1021566 [ use-skia-dawn ] Pixel_Video_VP9 [ Skip ]
crbug.com/1021566 [ use-skia-dawn ] Pixel_WebGLCopyImage [ Skip ]
crbug.com/1021566 [ use-skia-dawn ] Pixel_WebGLGreenTriangle_AA_Alpha [ Skip ]
......
content/test/gpu/gpu_tests/test_expectations/trace_test_expectations.txt
View file @ 1364e616
......@@ -58,6 +58,7 @@ crbug.com/1096431 [ fuchsia ] TraceTest_Video_MP4_FourColors_Rot_180 [ Skip ]
crbug.com/1096431 [ fuchsia ] TraceTest_Video_MP4_FourColors_Rot_270 [ Skip ]
crbug.com/1096431 [ fuchsia ] TraceTest_Video_MP4_FourColors_Rot_90 [ Skip ]
crbug.com/1096431 [ fuchsia ] TraceTest_Video_MP4_Rounded_Corner [ Skip ]
crbug.com/1096431 [ fuchsia ] TraceTest_Video_VP8_Incompatible_Stride [ Skip ]
crbug.com/1096431 [ fuchsia ] TraceTest_Video_VP9 [ Skip ]
crbug.com/1096431 [ fuchsia ] TraceTest_Video_VP9_DXVA [ Skip ]
......
media/renderers/DEPS
View file @ 1364e616
......@@ -7,6 +7,7 @@ include_rules = [
"+media/base/bind_to_current_loop.h",
"+media/base/speech_recognition_client.h",
"+third_party/khronos/GLES2",
"+third_party/khronos/GLES3",
"+ul/gl/gl_enums.h",
]
......
media/renderers/video_resource_updater.cc
View file @ 1364e616
......@@ -46,6 +46,7 @@
#include "media/video/half_float_maker.h"
#include "third_party/khronos/GLES2/gl2.h"
#include "third_party/khronos/GLES2/gl2ext.h"
#include "third_party/khronos/GLES3/gl3.h"
#include "third_party/libyuv/include/libyuv.h"
#include "third_party/skia/include/core/SkCanvas.h"
#include "ui/gfx/geometry/size_conversions.h"
......@@ -1174,20 +1175,28 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
// Data downshifting is needed if the resource bit depth is not enough.
const bool needs_bit_downshifting = bits_per_channel > resource_bit_depth;
// A copy to adjust strides is needed if those are different and both source
// and destination have the same bit depth.
const bool needs_stride_adaptation =
(bits_per_channel == resource_bit_depth) &&
(upload_image_stride != static_cast<size_t>(video_stride_bytes));
// We need to convert the incoming data if we're transferring to half float,
// if the need a bit downshift or if the strides need to be reconciled.
const bool needs_conversion = plane_resource_format == viz::LUMINANCE_F16 ||
needs_bit_downshifting ||
needs_stride_adaptation;
const bool needs_conversion =
plane_resource_format == viz::LUMINANCE_F16 || needs_bit_downshifting;
const uint8_t* pixels;
GLuint unpack_row_length = 0;
if (!needs_conversion) {
// Stride adaptation is needed if source and destination strides are
// different but they have the same bit depth.
const bool needs_stride_adaptation =
(bits_per_channel == resource_bit_depth) &&
(upload_image_stride != static_cast<size_t>(video_stride_bytes));
if (needs_stride_adaptation) {
const int bytes_per_element =
VideoFrame::BytesPerElement(video_frame->format(), i);
// Stride is aligned to VideoFrameLayout::kFrameAddressAlignment (32)
// which should be divisible by pixel size for YUV formats (1, 2 or 4).
DCHECK_EQ(video_stride_bytes % bytes_per_element, 0);
// Unpack row length is in pixels not bytes.
unpack_row_length = video_stride_bytes / bytes_per_element;
}
pixels = video_frame->data(i);
} else {
// Avoid malloc for each frame/plane if possible.
......@@ -1218,14 +1227,7 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
video_stride_bytes / 2, upload_pixels_.get(), upload_image_stride,
scale, bytes_per_row, resource_size_pixels.height());
} else {
// Make a copy to reconcile stride, size and format being equal.
DCHECK(needs_stride_adaptation);
DCHECK(plane_resource_format == viz::LUMINANCE_8 ||
plane_resource_format == viz::RED_8);
libyuv::CopyPlane(video_frame->data(i), video_stride_bytes,
upload_pixels_.get(), upload_image_stride,
resource_size_pixels.width(),
resource_size_pixels.height());
NOTREACHED();
}
pixels = upload_pixels_.get();
......@@ -1238,12 +1240,16 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
DCHECK(GLSupportsFormat(plane_resource_format));
{
HardwarePlaneResource::ScopedTexture scope(gl, plane_resource);
gl->BindTexture(plane_resource->texture_target(), scope.texture_id());
gl->PixelStorei(GL_UNPACK_ROW_LENGTH, unpack_row_length);
gl->TexSubImage2D(plane_resource->texture_target(), 0, 0, 0,
resource_size_pixels.width(),
resource_size_pixels.height(),
GLDataFormat(plane_resource_format),
GLDataType(plane_resource_format), pixels);
gl->PixelStorei(GL_UNPACK_ROW_LENGTH, 0);
}
plane_resource->SetUniqueId(video_frame->unique_id(), i);
......
media/test/data/README.md
View file @ 1364e616
......@@ -268,6 +268,13 @@ a rotation of 180 degrees in mp4 meta data.
Actual video frames are the same as four-colors.mp4, except it specifies
a rotation of 270 degrees in mp4 meta data.
#### four-colors-vp8-incompatible-stride.webm
A 962x540 vp8 video with 4 color blocks (Y,R,G,B) in every frame with a GL
incompatible stride. Converted from four-colors.mp4 using ffmpeg:
```
ffmpeg -i four-colors.mp4 -vf "pad=w=962,format=yuv420p" -c:v libvpx four-colors-vp8-incompatible-stride.webm
```
#### four-colors-vp9.webm
A 960x540 vp9 video with 4 color blocks (Y,R,G,B) in every frame. This is
converted from four-colors.mp4 by ffmpeg.
......
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