Use NV12 DXGI format when using DXVA2 with D3D11

RESOLVED FIXED in Firefox 55

Status

()

Core
Audio/Video: Playback
RESOLVED FIXED
11 months ago
7 months ago

People

(Reporter: bas, Assigned: bas)

Tracking

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Assignee)

Description

11 months ago
This will save us significantly on sampler load on the GPU.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 3

11 months ago
mozreview-review
Comment on attachment 8838386 [details]
Bug 1340398 - Part 1: Allow SyncObject usage for different devices.

https://reviewboard.mozilla.org/r/113324/#review114834
Attachment #8838386 - Flags: review?(matt.woodrow) → review+

Comment 4

11 months ago
mozreview-review
Comment on attachment 8838386 [details]
Bug 1340398 - Part 1: Allow SyncObject usage for different devices.

https://reviewboard.mozilla.org/r/113324/#review114840

::: gfx/layers/d3d11/CompositorD3D11.cpp:876
(Diff revision 2)
>  
>        SetSamplerForSamplingFilter(texturedEffect->mSamplingFilter);
>      }
>      break;
> +  case EffectTypes::NV12:
> +    {

nit: placement of { isn't consistent with the rest here.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 10

11 months ago
Matt, note it's mistakingly suggesting you reviewed part 1. My reorder confused reviewboard :)

Comment 11

11 months ago
mozreview-review
Comment on attachment 8838386 [details]
Bug 1340398 - Part 1: Allow SyncObject usage for different devices.

https://reviewboard.mozilla.org/r/113324/#review118424

::: gfx/layers/client/TextureClient.h:107
(Diff revision 4)
>  public:
>    MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(SyncObject)
>    virtual ~SyncObject() { }
>  
> -  static already_AddRefed<SyncObject> CreateSyncObject(SyncHandle aHandle);
> +  static already_AddRefed<SyncObject> CreateSyncObject(SyncHandle aHandle,
> +                                                       void* aNativeData = nullptr);

Can we just make this extra parameter #ifdef XP_WIN and then use the real type?

static_cast always makes me a little bit sad.

Comment 12

11 months ago
mozreview-review
Comment on attachment 8842985 [details]
Bug 1340398 - Part 3: Use NV12 format by default.

https://reviewboard.mozilla.org/r/116724/#review118436

::: gfx/layers/D3D11ShareHandleImage.cpp:196
(Diff revision 1)
>  already_AddRefed<TextureClient>
>  D3D11RecycleAllocator::CreateOrRecycleClient(gfx::SurfaceFormat aFormat,
>                                               const gfx::IntSize& aSize)
>  {
> +  TextureAllocationFlags allocFlags = TextureAllocationFlags::ALLOC_DEFAULT;
> +  if (gfxPrefs::PDMWMFUseSyncTexture()) {

This can be called off the main thread, right?

Should probably cache the value in case it changes between calls.
Attachment #8842985 - Flags: review?(matt.woodrow) → review+

Comment 13

11 months ago
mozreview-review
Comment on attachment 8842403 [details]
Bug 1340398 - Part 2: Support NV12 texture hosts in CompositorD3D11.

https://reviewboard.mozilla.org/r/116270/#review118438
Attachment #8842403 - Flags: review?(matt.woodrow) → review+
(Assignee)

Comment 14

11 months ago
mozreview-review
Comment on attachment 8842985 [details]
Bug 1340398 - Part 3: Use NV12 format by default.

https://reviewboard.mozilla.org/r/116724/#review119604

::: gfx/layers/D3D11ShareHandleImage.cpp:196
(Diff revision 1)
>  already_AddRefed<TextureClient>
>  D3D11RecycleAllocator::CreateOrRecycleClient(gfx::SurfaceFormat aFormat,
>                                               const gfx::IntSize& aSize)
>  {
> +  TextureAllocationFlags allocFlags = TextureAllocationFlags::ALLOC_DEFAULT;
> +  if (gfxPrefs::PDMWMFUseSyncTexture()) {

I'll make this pref none-live.

Comment 15

11 months ago
Pushed by bschouten@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/588942cf8807
Part 1: Allow SyncObject usage for different devices. r=mattwoodrow
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8af64e23154
Part 2: Support NV12 texture hosts in CompositorD3D11. r=mattwoodrow
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d0a9bb0a642
Part 3: Use NV12 format by default. r=mattwoodrow
(Assignee)

Comment 17

11 months ago
(In reply to Wes Kocher (:KWierso) from comment #16)
> I had to back these out for static build failures like
> https://treeherder.mozilla.org/logviewer.html#?job_id=82256847&repo=mozilla-
> inbound
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/db74935c78fa

Ugh, I don't even think that's actually wrong, it's just forcing me to make the code uglier. Oh well.
Flags: needinfo?(bas)

Comment 18

11 months ago
Pushed by bschouten@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b94a413fe46a
Part 1: Allow SyncObject usage for different devices. r=mattwoodrow
https://hg.mozilla.org/integration/mozilla-inbound/rev/be80dc74a965
Part 2: Support NV12 texture hosts in CompositorD3D11. r=mattwoodrow
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d4603ebe97e
Part 3: Use NV12 format by default. r=mattwoodrow

Comment 19

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b94a413fe46a
https://hg.mozilla.org/mozilla-central/rev/be80dc74a965
https://hg.mozilla.org/mozilla-central/rev/0d4603ebe97e
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1346267

Updated

10 months ago
Depends on: 1346670

Updated

10 months ago
Depends on: 1352016
Depends on: 1374936

Updated

7 months ago
Depends on: 1375461
You need to log in before you can comment on or make changes to this bug.