Skip to content

GitLab

T
tangled
Commit 10630b8a 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 1364e616

Source alignment is taken into account and a new pixel test is added.

Original change's description:
> 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/+/2390958
> Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#803960}

Bug: 1077211
Change-Id: Ie2bf9653751bed6491a902a133b3b07b7b700f49
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2402192
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: default avatarZhenyao Mo <zmo@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808173}
parent 9865da28
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 125 additions and 19 deletions
content/test/BUILD.gn
View file @ 10630b8a
...@@ -671,6 +671,7 @@ group("telemetry_gpu_integration_test_support") { ...@@ -671,6 +671,7 @@ group("telemetry_gpu_integration_test_support") {
"//media/test/data/four-colors-rot-270.mp4", "//media/test/data/four-colors-rot-270.mp4",
"//media/test/data/four-colors-vp9.webm", "//media/test/data/four-colors-vp9.webm",
"//media/test/data/four-colors-vp9-i420a.webm", "//media/test/data/four-colors-vp9-i420a.webm",
"//media/test/data/four-colors-incompatible-stride.y4m",
# For power # For power
"//media/test/data/bear-1280x720.mp4", "//media/test/data/bear-1280x720.mp4",
......
content/test/data/gpu/pixel_video_media_stream_incompatible_stride.html 0 → 100644
View file @ 10630b8a
<!DOCTYPE HTML>
<html>
<head>
<title>Media Stream Incompatible Stride Video test</title>
<style type="text/css">
.nomargin {
margin: 0px auto;
}
</style>
<script src="pixel_video_media_stream_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">
</video>
</div>
</body>
</html>
content/test/data/gpu/pixel_video_media_stream_test.js 0 → 100644
View file @ 10630b8a
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
var video;
// Some videos are less than 60 fps, so actual video frame presentations
// could be much less than 30.
var g_swaps_before_success = 30
async function main() {
video = document.getElementById("video");
video.loop = true;
video.requestVideoFrameCallback(waitForVideoToPlay);
video.srcObject = await navigator.mediaDevices.getUserMedia({video: true});
video.play();
}
function waitForVideoToPlay() {
chrome.gpuBenchmarking.addSwapCompletionEventListener(
waitForSwapsToComplete);
}
function waitForSwapsToComplete() {
g_swaps_before_success--;
if (g_swaps_before_success > 0) {
chrome.gpuBenchmarking.addSwapCompletionEventListener(
waitForSwapsToComplete);
} else {
domAutomationController.send("SUCCESS");
}
}
content/test/gpu/gpu_tests/pixel_test_pages.py
View file @ 10630b8a
...@@ -5,8 +5,11 @@ ...@@ -5,8 +5,11 @@
# This is more akin to a .pyl/JSON file, so it's expected to be long. # This is more akin to a .pyl/JSON file, so it's expected to be long.
# pylint: disable=too-many-lines # pylint: disable=too-many-lines
import os
from gpu_tests import common_browser_args as cba from gpu_tests import common_browser_args as cba
from gpu_tests import skia_gold_matching_algorithms as algo from gpu_tests import skia_gold_matching_algorithms as algo
from gpu_tests import path_util
CRASH_TYPE_GPU = 'gpu' CRASH_TYPE_GPU = 'gpu'
...@@ -110,6 +113,14 @@ def CopyPagesWithNewBrowserArgsAndPrefix(pages, browser_args, prefix): ...@@ -110,6 +113,14 @@ def CopyPagesWithNewBrowserArgsAndPrefix(pages, browser_args, prefix):
] ]
def GetMediaStreamTestBrowserArgs(media_stream_source_relpath):
return [
'--use-fake-device-for-media-stream', '--use-fake-ui-for-media-stream',
'--use-file-for-fake-video-capture=' +
os.path.join(path_util.GetChromiumSrcDir(), media_stream_source_relpath)
]
class PixelTestPages(object): class PixelTestPages(object):
@staticmethod @staticmethod
def DefaultPages(base_name): def DefaultPages(base_name):
...@@ -224,6 +235,13 @@ class PixelTestPages(object): ...@@ -224,6 +235,13 @@ class PixelTestPages(object):
max_different_pixels=31100, max_different_pixels=31100,
pixel_delta_threshold=30, pixel_delta_threshold=30,
edge_threshold=250)), edge_threshold=250)),
PixelTestPage(
'pixel_video_media_stream_incompatible_stride.html',
base_name + '_Video_Media_Stream_Incompatible_Stride',
browser_args=GetMediaStreamTestBrowserArgs(
'media/test/data/four-colors-incompatible-stride.y4m'),
test_rect=[0, 0, 240, 135],
matching_algorithm=VERY_PERMISSIVE_SOBEL_ALGO),
# The MP4 contains H.264 which is primarily hardware decoded on bots. # The MP4 contains H.264 which is primarily hardware decoded on bots.
PixelTestPage( PixelTestPage(
......
content/test/gpu/gpu_tests/test_expectations/pixel_expectations.txt
View file @ 10630b8a
...@@ -79,6 +79,7 @@ crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_MP4_FourColors_Rot_90 ...@@ -79,6 +79,7 @@ crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_MP4_FourColors_Rot_90
crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_MP4 [ Skip ] crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_MP4 [ Skip ]
crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_VP9 [ 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 ] crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_VP9_DXVA [ Skip ]
crbug.com/1084367 [ fuchsia no-use-skia-dawn ] Pixel_Video_Media_Stream_Incompatible_Stride [ Skip ]
# Fuchsia issues # Fuchsia issues
crbug.com/1115673 [ fuchsia no-use-skia-dawn ] Pixel_WebGLCopyImage [ Skip ] crbug.com/1115673 [ fuchsia no-use-skia-dawn ] Pixel_WebGLCopyImage [ Skip ]
...@@ -363,6 +364,7 @@ crbug.com/1021566 [ use-skia-dawn skia-renderer ] Pixel_Video_MP4_FourColors_Rot ...@@ -363,6 +364,7 @@ crbug.com/1021566 [ use-skia-dawn skia-renderer ] Pixel_Video_MP4_FourColors_Rot
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_270 [ Skip ]
crbug.com/1021566 [ use-skia-dawn skia-renderer ] Pixel_Video_MP4_FourColors_Rot_90 [ Skip ] crbug.com/1021566 [ use-skia-dawn skia-renderer ] Pixel_Video_MP4_FourColors_Rot_90 [ Skip ]
crbug.com/1021566 [ use-skia-dawn ] Pixel_Video_VP9 [ Skip ] crbug.com/1021566 [ use-skia-dawn ] Pixel_Video_VP9 [ Skip ]
crbug.com/1021566 [ use-skia-dawn ] Pixel_Video_Media_Stream_Incompatible_Stride [ Skip ]
crbug.com/1021566 [ use-skia-dawn ] Pixel_WebGLCopyImage [ Skip ] crbug.com/1021566 [ use-skia-dawn ] Pixel_WebGLCopyImage [ Skip ]
crbug.com/1021566 [ use-skia-dawn ] Pixel_WebGLGreenTriangle_AA_Alpha [ Skip ] crbug.com/1021566 [ use-skia-dawn ] Pixel_WebGLGreenTriangle_AA_Alpha [ Skip ]
crbug.com/1021566 [ use-skia-dawn ] Pixel_WebGLGreenTriangle_AA_NoAlpha [ Skip ] crbug.com/1021566 [ use-skia-dawn ] Pixel_WebGLGreenTriangle_AA_NoAlpha [ Skip ]
...@@ -387,3 +389,6 @@ crbug.com/1086690 [ chromeos ] Pixel_WebGLSadCanvas [ Skip ] ...@@ -387,3 +389,6 @@ crbug.com/1086690 [ chromeos ] Pixel_WebGLSadCanvas [ Skip ]
# Likely produces a Dawn validation errors that makes rendering skipped. # Likely produces a Dawn validation errors that makes rendering skipped.
crbug.com/1097735 [ use-skia-dawn ] Pixel_PaintWorkletTransform [ Failure ] crbug.com/1097735 [ use-skia-dawn ] Pixel_PaintWorkletTransform [ Failure ]
# Cannot use a file in source directory on host for fake video capture device on Android
crbug.com/1077211 [ android no-use-skia-dawn ] Pixel_Video_Media_Stream_Incompatible_Stride [ Skip ]
content/test/gpu/gpu_tests/test_expectations/trace_test_expectations.txt
View file @ 10630b8a
...@@ -60,6 +60,7 @@ crbug.com/1096431 [ fuchsia ] TraceTest_Video_MP4_FourColors_Rot_90 [ Skip ] ...@@ -60,6 +60,7 @@ 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_MP4_Rounded_Corner [ Skip ]
crbug.com/1096431 [ fuchsia ] TraceTest_Video_VP9 [ Skip ] crbug.com/1096431 [ fuchsia ] TraceTest_Video_VP9 [ Skip ]
crbug.com/1096431 [ fuchsia ] TraceTest_Video_VP9_DXVA [ Skip ] crbug.com/1096431 [ fuchsia ] TraceTest_Video_VP9_DXVA [ Skip ]
crbug.com/1096431 [ fuchsia ] TraceTest_Video_Media_Stream_Incompatible_Stride [ Skip ]
# Skip unaccelerated canvas test since we don't use swap chains without GPU compositing. # Skip unaccelerated canvas test since we don't use swap chains without GPU compositing.
crbug.com/1009860 [ win10 ] SwapChainTraceTest_CanvasUnacceleratedLowLatency2D [ Skip ] crbug.com/1009860 [ win10 ] SwapChainTraceTest_CanvasUnacceleratedLowLatency2D [ Skip ]
...@@ -129,3 +130,6 @@ crbug.com/1058255 [ fuchsia ] TraceTest_WebGLGreenTriangle_AA_Alpha [ Skip ] ...@@ -129,3 +130,6 @@ crbug.com/1058255 [ fuchsia ] TraceTest_WebGLGreenTriangle_AA_Alpha [ Skip ]
[ fuchsia ] DeviceTraceTest_RepeatedWebGLTo2D_SoftwareCompositing [ Skip ] [ fuchsia ] DeviceTraceTest_RepeatedWebGLTo2D_SoftwareCompositing [ Skip ]
[ fuchsia ] DeviceTraceTest_Canvas2DTabSwitch_SoftwareCompositing [ Skip ] [ fuchsia ] DeviceTraceTest_Canvas2DTabSwitch_SoftwareCompositing [ Skip ]
[ fuchsia ] DeviceTraceTest_WebGLReadPixelsTabSwitch_SoftwareCompositing [ Skip ] [ fuchsia ] DeviceTraceTest_WebGLReadPixelsTabSwitch_SoftwareCompositing [ Skip ]
# Cannot use a file in Chromium checkout for fake video capture device on Android
crbug.com/1077211 [ android ] TraceTest_Video_Media_Stream_Incompatible_Stride [ Skip ]
media/renderers/DEPS
View file @ 10630b8a
...@@ -7,6 +7,7 @@ include_rules = [ ...@@ -7,6 +7,7 @@ include_rules = [
"+media/base/bind_to_current_loop.h", "+media/base/bind_to_current_loop.h",
"+media/base/speech_recognition_client.h", "+media/base/speech_recognition_client.h",
"+third_party/khronos/GLES2", "+third_party/khronos/GLES2",
"+third_party/khronos/GLES3",
"+ul/gl/gl_enums.h", "+ul/gl/gl_enums.h",
] ]
......
media/renderers/video_resource_updater.cc
View file @ 10630b8a
...@@ -46,6 +46,7 @@ ...@@ -46,6 +46,7 @@
#include "media/video/half_float_maker.h" #include "media/video/half_float_maker.h"
#include "third_party/khronos/GLES2/gl2.h" #include "third_party/khronos/GLES2/gl2.h"
#include "third_party/khronos/GLES2/gl2ext.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/libyuv/include/libyuv.h"
#include "third_party/skia/include/core/SkCanvas.h" #include "third_party/skia/include/core/SkCanvas.h"
#include "ui/gfx/geometry/size_conversions.h" #include "ui/gfx/geometry/size_conversions.h"
...@@ -1163,10 +1164,12 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes( ...@@ -1163,10 +1164,12 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
const size_t bytes_per_row = const size_t bytes_per_row =
viz::ResourceSizes::CheckedWidthInBytes<size_t>( viz::ResourceSizes::CheckedWidthInBytes<size_t>(
resource_size_pixels.width(), plane_resource_format); resource_size_pixels.width(), plane_resource_format);
// Use 4-byte row alignment (OpenGL default) for upload performance. // Use 4-byte row alignment (OpenGL default) for upload performance.
// Assuming that GL_UNPACK_ALIGNMENT has not changed from default. // Assuming that GL_UNPACK_ALIGNMENT has not changed from default.
const size_t upload_image_stride = constexpr size_t kDefaultUnpackAlignment = 4;
cc::MathUtil::CheckedRoundUp<size_t>(bytes_per_row, 4u); const size_t upload_image_stride = cc::MathUtil::CheckedRoundUp<size_t>(
bytes_per_row, kDefaultUnpackAlignment);
const size_t resource_bit_depth = const size_t resource_bit_depth =
static_cast<size_t>(viz::BitsPerPixel(plane_resource_format)); static_cast<size_t>(viz::BitsPerPixel(plane_resource_format));
...@@ -1174,20 +1177,35 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes( ...@@ -1174,20 +1177,35 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
// Data downshifting is needed if the resource bit depth is not enough. // Data downshifting is needed if the resource bit depth is not enough.
const bool needs_bit_downshifting = bits_per_channel > resource_bit_depth; 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, // 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. // if the need a bit downshift or if the strides need to be reconciled.
const bool needs_conversion = plane_resource_format == viz::LUMINANCE_F16 || const bool needs_conversion =
needs_bit_downshifting || plane_resource_format == viz::LUMINANCE_F16 || needs_bit_downshifting;
needs_stride_adaptation;
constexpr size_t kDefaultUnpackRowLength = 0;
GLuint unpack_row_length = kDefaultUnpackRowLength;
GLuint unpack_alignment = kDefaultUnpackAlignment;
const uint8_t* pixels; const uint8_t* pixels;
if (!needs_conversion) { 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;
// Use a non-standard alignment only if necessary.
if (video_stride_bytes % kDefaultUnpackAlignment != 0)
unpack_alignment = bytes_per_element;
}
pixels = video_frame->data(i); pixels = video_frame->data(i);
} else { } else {
// Avoid malloc for each frame/plane if possible. // Avoid malloc for each frame/plane if possible.
...@@ -1218,14 +1236,7 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes( ...@@ -1218,14 +1236,7 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
video_stride_bytes / 2, upload_pixels_.get(), upload_image_stride, video_stride_bytes / 2, upload_pixels_.get(), upload_image_stride,
scale, bytes_per_row, resource_size_pixels.height()); scale, bytes_per_row, resource_size_pixels.height());
} else { } else {
// Make a copy to reconcile stride, size and format being equal. NOTREACHED();
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());
} }
pixels = upload_pixels_.get(); pixels = upload_pixels_.get();
...@@ -1238,12 +1249,18 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes( ...@@ -1238,12 +1249,18 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
DCHECK(GLSupportsFormat(plane_resource_format)); DCHECK(GLSupportsFormat(plane_resource_format));
{ {
HardwarePlaneResource::ScopedTexture scope(gl, plane_resource); HardwarePlaneResource::ScopedTexture scope(gl, plane_resource);
gl->BindTexture(plane_resource->texture_target(), scope.texture_id()); gl->BindTexture(plane_resource->texture_target(), scope.texture_id());
gl->PixelStorei(GL_UNPACK_ROW_LENGTH, unpack_row_length);
gl->PixelStorei(GL_UNPACK_ALIGNMENT, unpack_alignment);
gl->TexSubImage2D(plane_resource->texture_target(), 0, 0, 0, gl->TexSubImage2D(plane_resource->texture_target(), 0, 0, 0,
resource_size_pixels.width(), resource_size_pixels.width(),
resource_size_pixels.height(), resource_size_pixels.height(),
GLDataFormat(plane_resource_format), GLDataFormat(plane_resource_format),
GLDataType(plane_resource_format), pixels); GLDataType(plane_resource_format), pixels);
gl->PixelStorei(GL_UNPACK_ROW_LENGTH, kDefaultUnpackRowLength);
gl->PixelStorei(GL_UNPACK_ALIGNMENT, kDefaultUnpackAlignment);
} }
plane_resource->SetUniqueId(video_frame->unique_id(), i); plane_resource->SetUniqueId(video_frame->unique_id(), i);
......
media/test/data/README.md
View file @ 10630b8a
...@@ -268,6 +268,13 @@ a rotation of 180 degrees in mp4 meta data. ...@@ -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 Actual video frames are the same as four-colors.mp4, except it specifies
a rotation of 270 degrees in mp4 meta data. a rotation of 270 degrees in mp4 meta data.
#### four-colors-incompatible-stride.y4m
A 962x540 raw YUV single frame video with 4 color blocks (Y,R,G,B) and a GL
incompatible stride. Converted from four-colors.mp4 using ffmpeg:
```
ffmpeg -i four-colors.mp4 -vf "scale=w=962:h=540,format=yuv420p" -frames:v 1 four-colors-incompatible-stride.y4m
```
#### four-colors-vp9.webm #### four-colors-vp9.webm
A 960x540 vp9 video with 4 color blocks (Y,R,G,B) in every frame. This is A 960x540 vp9 video with 4 color blocks (Y,R,G,B) in every frame. This is
converted from four-colors.mp4 by ffmpeg. converted from four-colors.mp4 by ffmpeg.
......
media/test/data/four-colors-incompatible-stride.y4m 0 → 100644
View file @ 10630b8a
This diff is collapsed.
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