New issue
Advanced search Search tips

Issue 832917 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocking:
issue 827649



Sign in to add a comment

d3d11 zero copy: use decoderconfig to choose between D3D11 / VDA decoders

Project Member Reported by liber...@chromium.org, Apr 13 2018

Issue description

when VDAVideoDecoder replaces GpuVideoDecoder, we'll want to pick between D3D11 and VDA in GpuMojoMediaClient.  This will save a round-trip to the client, rather than creating both and letting init failure inform the decoder selector.

 
dale had a good idea -- create (or maybe reuse -- not sure if it exists yet) a VideoDecoder that wraps a primary and fallback decoder on the gpu side.  then, it can decide during init which one to use based the init_cb results.

no plumbing needed.
We can do this with no wrapper, by moving the VideoDecoder construction to MojoVideoDecoderService::Initialize().

I'd prefer doing it at Construct() time as an overall code health thing (for example, it would make caching on the renderer side more effective), but performance-wise they are similar because MojoVideoDecoder calls Construct() and Initialize() at the same time.
I take that back; there is no obvious code health improvement having this in Construct(). MojoVideoDecoder doesn't do anything until Initialize() is called anyway.
i was thinking a bit more about this.  we might still want to handle the case where d3d11::Initialize fails unexpectedly, and fall back to VDAVideoDecoder.  this is much easier to do on the gpu side, if we want to avoid the ipc associated with making two MojoVideoDecoder instances.

what i don't know is if there are any cases where d3d11 would advertise that it will work, but then fail and dxva would still work.

however, if there are, then i think we need to do either the wrapper or the MVDS::Initialize thing you suggested (which i like better, by the way).
didn't refresh to see c#3 before c#4.

okay, then, let's do the MJVDS::Initialize thing you suggested.  it's way simpler than the plumbing in any case.
Cc: liber...@chromium.org
Owner: tmathmeyer@chromium.org
Status: Started (was: Assigned)
after writing up both options, the fallback video decoder ends up being simpler in practice.

=> tmathmeyer, who's working on this currently.
Project Member

Comment 7 by bugdroid1@chromium.org, May 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/033eec0f3c7e44901a1553d219db028db9e2daea

commit 033eec0f3c7e44901a1553d219db028db9e2daea
Author: Ted Meyer <tmathmeyer@chromium.org>
Date: Fri May 04 01:15:48 2018

Adds FallbackVideoDecoder for wrapping two decoders

Allows a preferred and fallback decoder to be sent cross-thread and
initialized without requiring extra thread hops back to report the
status of an intermediate failure.

Bug:  832917 
Change-Id: Ib9e014a746e38588ce285153230511b33db8e47b
Reviewed-on: https://chromium-review.googlesource.com/1023115
Commit-Queue: Frank Liberato <liberato@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555948}
[modify] https://crrev.com/033eec0f3c7e44901a1553d219db028db9e2daea/media/base/BUILD.gn
[add] https://crrev.com/033eec0f3c7e44901a1553d219db028db9e2daea/media/base/fallback_video_decoder.cc
[add] https://crrev.com/033eec0f3c7e44901a1553d219db028db9e2daea/media/base/fallback_video_decoder.h
[add] https://crrev.com/033eec0f3c7e44901a1553d219db028db9e2daea/media/base/fallback_video_decoder_unittest.cc
[modify] https://crrev.com/033eec0f3c7e44901a1553d219db028db9e2daea/media/base/mock_filters.h

You're the owner of a Pri-1 M-69 chrome media issue. M-69 is now in beta and will ship to stable in coming weeks. See go/chromeschedule. Please work on resolving your issue ASAP if it needs fixing for the M-69 branch.

Pri-1 means the work is required for the branch. Alternatively, update the milestone to M-70 or remove the milestone and drop the priority to P-3.
Labels: -Pri-1 Pri-2
Labels: -M-69 M-70
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e04db346d28a074f4fcf1f4f5fbf757fd071b85

commit 0e04db346d28a074f4fcf1f4f5fbf757fd071b85
Author: Frank Liberato <liberato@chromium.org>
Date: Tue Oct 23 00:01:07 2018

Make D3D11VideoDecoder fall back to VDAVideoDecoder.

To try to reduce time to first frame regressions, this CL makes
D3D11VideoDecoder fall back to VDAVideoDecoder via
FallbacKVideoDecoder.  This prevents a round trip or two between the
GPU and renderer processes.

Bug:  832917 
Change-Id: I2b3df44805fd950328beb6db88590718550630d1
Reviewed-on: https://chromium-review.googlesource.com/c/1294810
Commit-Queue: Frank Liberato <liberato@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601789}
[modify] https://crrev.com/0e04db346d28a074f4fcf1f4f5fbf757fd071b85/media/mojo/services/gpu_mojo_media_client.cc
[modify] https://crrev.com/0e04db346d28a074f4fcf1f4f5fbf757fd071b85/media/renderers/default_decoder_factory.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad0acbffaa52730b8aed065315347228c1bddd8b

commit ad0acbffaa52730b8aed065315347228c1bddd8b
Author: Frank Liberato <liberato@chromium.org>
Date: Tue Oct 23 17:47:05 2018

Revert "Make D3D11VideoDecoder fall back to VDAVideoDecoder."

This reverts commit 0e04db346d28a074f4fcf1f4f5fbf757fd071b85.

Reason for revert: crbug.com/898080

Original change's description:
> Make D3D11VideoDecoder fall back to VDAVideoDecoder.
> 
> To try to reduce time to first frame regressions, this CL makes
> D3D11VideoDecoder fall back to VDAVideoDecoder via
> FallbacKVideoDecoder.  This prevents a round trip or two between the
> GPU and renderer processes.
> 
> Bug:  832917 
> Change-Id: I2b3df44805fd950328beb6db88590718550630d1
> Reviewed-on: https://chromium-review.googlesource.com/c/1294810
> Commit-Queue: Frank Liberato <liberato@chromium.org>
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#601789}

TBR=sandersd@chromium.org,liberato@chromium.org

Change-Id: Iddb9909e9c024899b640f667e25350c380ee9a0a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  832917 
Reviewed-on: https://chromium-review.googlesource.com/c/1297079
Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602007}
[modify] https://crrev.com/ad0acbffaa52730b8aed065315347228c1bddd8b/media/mojo/services/gpu_mojo_media_client.cc
[modify] https://crrev.com/ad0acbffaa52730b8aed065315347228c1bddd8b/media/renderers/default_decoder_factory.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75ed09d97791b36f4ef29eb797cad9bda5b0971d

commit 75ed09d97791b36f4ef29eb797cad9bda5b0971d
Author: liberato@chromium.org <liberato@chromium.org>
Date: Wed Oct 24 01:57:05 2018

[Reland] Make D3D11VideoDecoder fall back to VDAVideoDecoder.

To try to reduce time to first frame regressions, this CL makes
D3D11VideoDecoder fall back to VDAVideoDecoder via
FallbacKVideoDecoder.  This prevents a round trip or two between the
GPU and renderer processes.

This relands https://chromium-review.googlesource.com/c/1294810 with
a fix to prevent deleting the preferred decoder during a failed init
callback.  Instead, it posts destruction.

Bug:  832917 
Change-Id: I24173ef8c4005961ed8ec0a9125b136b7267e6a7
Reviewed-on: https://chromium-review.googlesource.com/c/1297006
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602215}
[modify] https://crrev.com/75ed09d97791b36f4ef29eb797cad9bda5b0971d/media/base/fallback_video_decoder.cc
[modify] https://crrev.com/75ed09d97791b36f4ef29eb797cad9bda5b0971d/media/base/fallback_video_decoder_unittest.cc
[modify] https://crrev.com/75ed09d97791b36f4ef29eb797cad9bda5b0971d/media/mojo/services/gpu_mojo_media_client.cc
[modify] https://crrev.com/75ed09d97791b36f4ef29eb797cad9bda5b0971d/media/renderers/default_decoder_factory.cc

Status: Fixed (was: Started)

Sign in to add a comment