Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(113)

Issue 1851713002: Add support for detecting the maximum resolution supported by the GPU for H.264 videos. (Closed)

Created:
4 years, 8 months ago by ananta
Modified:
4 years, 8 months ago
CC:
chromium-reviews, posciak+watch_chromium.org, jam, mcasas+watch_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, piman+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add support for detecting the maximum resolution supported by the GPU for H.264 videos. Media foundation on Windows 8.1+ supports resolutions upto 4096 x 2304. If the underlying GPU does not support the higher resolutions, then it goes back to software decoding. While we want to support higher H.264 resolutions for H/W decoding, we don't want to do this, if the underlying driver does not support the same. To detect that, we attempt to create a raw DXVA decoder device for the desired configuration and thus detect the maximum resolution. We maintain a list of predefined resolutions, starting from 1920 x 1088 and going upto 4096 x 2304 and try and create raw devices for these resolutions. This enables us to support resolutions of upto 4K depending on whether the GPU supports it. BUG=573565 Committed: https://crrev.com/3f07acd53d22dcd20410d193f7b6d934e794109a Cr-Commit-Position: refs/heads/master@{#384736}

Patch Set 1 #

Patch Set 2 : Revert debugging changes #

Patch Set 3 : Remove newline #

Total comments: 6

Patch Set 4 : Address review comments #

Patch Set 5 : Revert changes to move D3D11 device creation to a helper #

Patch Set 6 : Added TRACE_EVENT calls to measure time #

Total comments: 2

Patch Set 7 : Remove CHECKs #

Total comments: 2

Patch Set 8 : Add scopes around TRACE_EVENT calls #

Patch Set 9 : Fix indentation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+219 lines, -32 lines) Patch
M content/common/gpu/media/dxva_video_decode_accelerator_win.h View 1 2 3 4 2 chunks +12 lines, -0 lines 0 comments Download
M content/common/gpu/media/dxva_video_decode_accelerator_win.cc View 1 2 3 4 5 6 7 8 5 chunks +207 lines, -32 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 18 (4 generated)
ananta
4 years, 8 months ago (2016-04-01 01:02:34 UTC) #2
jbauman
https://codereview.chromium.org/1851713002/diff/40001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right): https://codereview.chromium.org/1851713002/diff/40001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc#newcode1403 content/common/gpu/media/dxva_video_decode_accelerator_win.cc:1403: switches::kDisableD3D11)) { Probably don't need to check the command-line ...
4 years, 8 months ago (2016-04-01 01:23:49 UTC) #3
ananta
https://codereview.chromium.org/1851713002/diff/40001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right): https://codereview.chromium.org/1851713002/diff/40001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc#newcode1403 content/common/gpu/media/dxva_video_decode_accelerator_win.cc:1403: switches::kDisableD3D11)) { On 2016/04/01 01:23:49, jbauman wrote: > Probably ...
4 years, 8 months ago (2016-04-01 01:37:06 UTC) #4
sandersd (OOO until July 31)
https://codereview.chromium.org/1851713002/diff/40001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right): https://codereview.chromium.org/1851713002/diff/40001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc#newcode1325 content/common/gpu/media/dxva_video_decode_accelerator_win.cc:1325: std::pair<int, int> min_resolution = GetMinResolution(supported_profile); Please measure how much ...
4 years, 8 months ago (2016-04-01 01:38:52 UTC) #5
ananta
https://codereview.chromium.org/1851713002/diff/40001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right): https://codereview.chromium.org/1851713002/diff/40001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc#newcode1325 content/common/gpu/media/dxva_video_decode_accelerator_win.cc:1325: std::pair<int, int> min_resolution = GetMinResolution(supported_profile); On 2016/04/01 01:38:52, sandersd ...
4 years, 8 months ago (2016-04-01 03:28:16 UTC) #6
sandersd (OOO until July 31)
Looks okay % use of CHECK (two instances). https://codereview.chromium.org/1851713002/diff/100001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right): https://codereview.chromium.org/1851713002/diff/100001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc#newcode1466 content/common/gpu/media/dxva_video_decode_accelerator_win.cc:1466: CHECK(false); ...
4 years, 8 months ago (2016-04-01 21:02:54 UTC) #7
ananta
https://codereview.chromium.org/1851713002/diff/100001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right): https://codereview.chromium.org/1851713002/diff/100001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc#newcode1466 content/common/gpu/media/dxva_video_decode_accelerator_win.cc:1466: CHECK(false); On 2016/04/01 21:02:54, sandersd wrote: > This does ...
4 years, 8 months ago (2016-04-01 21:12:45 UTC) #8
sandersd (OOO until July 31)
lgtm
4 years, 8 months ago (2016-04-01 21:14:21 UTC) #9
jbauman
https://codereview.chromium.org/1851713002/diff/120001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right): https://codereview.chromium.org/1851713002/diff/120001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc#newcode1486 content/common/gpu/media/dxva_video_decode_accelerator_win.cc:1486: TRACE_EVENT0("gpu,startup", "GetMaxH264Resolution. H.264 guid search end"); FYI, this will ...
4 years, 8 months ago (2016-04-01 21:20:10 UTC) #10
ananta
https://codereview.chromium.org/1851713002/diff/120001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right): https://codereview.chromium.org/1851713002/diff/120001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc#newcode1486 content/common/gpu/media/dxva_video_decode_accelerator_win.cc:1486: TRACE_EVENT0("gpu,startup", "GetMaxH264Resolution. H.264 guid search end"); On 2016/04/01 21:20:09, ...
4 years, 8 months ago (2016-04-01 21:40:18 UTC) #11
jbauman
On 2016/04/01 21:40:18, ananta wrote: > https://codereview.chromium.org/1851713002/diff/120001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc > File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right): > > https://codereview.chromium.org/1851713002/diff/120001/content/common/gpu/media/dxva_video_decode_accelerator_win.cc#newcode1486 > ...
4 years, 8 months ago (2016-04-01 21:53:16 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1851713002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1851713002/160001
4 years, 8 months ago (2016-04-01 22:27:08 UTC) #15
commit-bot: I haz the power
Committed patchset #9 (id:160001)
4 years, 8 months ago (2016-04-01 23:59:41 UTC) #16
commit-bot: I haz the power
4 years, 8 months ago (2016-04-02 00:01:07 UTC) #18
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/3f07acd53d22dcd20410d193f7b6d934e794109a
Cr-Commit-Position: refs/heads/master@{#384736}

Powered by Google App Engine
This is Rietveld 408576698