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

Issue 11369229: Don't wait endlessly for flushing the batched queries sent to the GPU driver during H/W decode (Closed)

Created:
8 years, 1 month ago by ananta
Modified:
8 years, 1 month ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, apatrick_chromium, feature-media-reviews_chromium.org
Visibility:
Public.

Description

Don't wait endlessly for flushing the batched queries sent to the GPU driver during H/W decode We currently have a loop in the DXVA decoder on Windows 7 and above where we flush the queued D3D command bufers and wait for the operation to complete. This causes an infinite loop on certain multicore machines due to a bug in the IDirect3DQuery9::GetData call not returning the correct result. This seems to be a very old bug dating back to April 2008 http://us.generation-nt.com/idirect3dquery9-getdata-fails-return-ok-multi-core-cpus-help-26986872.html Fix for now is to have an upper limit of 10 iterations while we wait for the flush to complete. ld BUG=149139 R=apatrick Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=168093

Patch Set 1 #

Total comments: 13

Patch Set 2 : #

Patch Set 3 : #

Total comments: 4

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Total comments: 10

Patch Set 7 : #

Patch Set 8 : #

Total comments: 1

Patch Set 9 : #

Patch Set 10 : #

Patch Set 11 : #

Patch Set 12 : #

Patch Set 13 : #

Total comments: 4

Patch Set 14 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -7 lines) Patch
M content/common/gpu/media/dxva_video_decode_accelerator.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +25 lines, -7 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
ananta
8 years, 1 month ago (2012-11-13 22:32:40 UTC) #1
apatrick_chromium
lgtm
8 years, 1 month ago (2012-11-13 22:48:50 UTC) #2
Ami GONE FROM CHROMIUM
https://chromiumcodereview.appspot.com/11369229/diff/1/content/common/gpu/media/dxva_video_decode_accelerator.cc File content/common/gpu/media/dxva_video_decode_accelerator.cc (right): https://chromiumcodereview.appspot.com/11369229/diff/1/content/common/gpu/media/dxva_video_decode_accelerator.cc#newcode77 content/common/gpu/media/dxva_video_decode_accelerator.cc:77: // the batched queries to the driver. Why is ...
8 years, 1 month ago (2012-11-13 23:07:13 UTC) #3
ananta
https://chromiumcodereview.appspot.com/11369229/diff/1/content/common/gpu/media/dxva_video_decode_accelerator.cc File content/common/gpu/media/dxva_video_decode_accelerator.cc (right): https://chromiumcodereview.appspot.com/11369229/diff/1/content/common/gpu/media/dxva_video_decode_accelerator.cc#newcode77 content/common/gpu/media/dxva_video_decode_accelerator.cc:77: // the batched queries to the driver. On 2012/11/13 ...
8 years, 1 month ago (2012-11-13 23:17:21 UTC) #4
Ami GONE FROM CHROMIUM
I don't understand the rush, considering this was reported by exactly one person on stable ...
8 years, 1 month ago (2012-11-13 23:31:25 UTC) #5
ananta
On 2012/11/13 23:31:25, Ami Fischman wrote: > I don't understand the rush, considering this was ...
8 years, 1 month ago (2012-11-14 01:06:40 UTC) #6
ananta
https://chromiumcodereview.appspot.com/11369229/diff/5003/content/common/gpu/media/dxva_video_decode_accelerator.cc File content/common/gpu/media/dxva_video_decode_accelerator.cc (right): https://chromiumcodereview.appspot.com/11369229/diff/5003/content/common/gpu/media/dxva_video_decode_accelerator.cc#newcode78 content/common/gpu/media/dxva_video_decode_accelerator.cc:78: enum { kMaxiterationsForD3dFlush = 10 }; On 2012/11/13 23:31:25, ...
8 years, 1 month ago (2012-11-14 01:06:47 UTC) #7
Ami GONE FROM CHROMIUM
https://chromiumcodereview.appspot.com/11369229/diff/9002/content/common/gpu/media/dxva_video_decode_accelerator.cc File content/common/gpu/media/dxva_video_decode_accelerator.cc (right): https://chromiumcodereview.appspot.com/11369229/diff/9002/content/common/gpu/media/dxva_video_decode_accelerator.cc#newcode1 content/common/gpu/media/dxva_video_decode_accelerator.cc:1: // Copyright (c) 2012 The Chromium Authors. All rights ...
8 years, 1 month ago (2012-11-14 01:26:07 UTC) #8
apatrick_chromium
https://chromiumcodereview.appspot.com/11369229/diff/1/content/common/gpu/media/dxva_video_decode_accelerator.cc File content/common/gpu/media/dxva_video_decode_accelerator.cc (right): https://chromiumcodereview.appspot.com/11369229/diff/1/content/common/gpu/media/dxva_video_decode_accelerator.cc#newcode77 content/common/gpu/media/dxva_video_decode_accelerator.cc:77: // the batched queries to the driver. I expect ...
8 years, 1 month ago (2012-11-14 01:27:49 UTC) #9
ananta
https://chromiumcodereview.appspot.com/11369229/diff/9002/content/common/gpu/media/dxva_video_decode_accelerator.cc File content/common/gpu/media/dxva_video_decode_accelerator.cc (right): https://chromiumcodereview.appspot.com/11369229/diff/9002/content/common/gpu/media/dxva_video_decode_accelerator.cc#newcode1 content/common/gpu/media/dxva_video_decode_accelerator.cc:1: // Copyright (c) 2012 The Chromium Authors. All rights ...
8 years, 1 month ago (2012-11-14 03:05:42 UTC) #10
ananta
https://chromiumcodereview.appspot.com/11369229/diff/10002/content/common/gpu/media/dxva_video_decode_accelerator.cc File content/common/gpu/media/dxva_video_decode_accelerator.cc (right): https://chromiumcodereview.appspot.com/11369229/diff/10002/content/common/gpu/media/dxva_video_decode_accelerator.cc#newcode390 content/common/gpu/media/dxva_video_decode_accelerator.cc:390: hr = query_->Issue(D3DISSUE_END); Issuing the query upfront causes the ...
8 years, 1 month ago (2012-11-14 03:06:47 UTC) #11
Ami GONE FROM CHROMIUM
This change LGTM but I wonder whether it'll actually fix Derek's problem from the linked ...
8 years, 1 month ago (2012-11-14 17:13:10 UTC) #12
ananta
On 2012/11/14 17:13:10, Ami Fischman wrote: > This change LGTM but I wonder whether it'll ...
8 years, 1 month ago (2012-11-15 21:39:16 UTC) #13
Ami GONE FROM CHROMIUM
LGTM % nits https://chromiumcodereview.appspot.com/11369229/diff/31002/content/common/gpu/media/dxva_video_decode_accelerator.cc File content/common/gpu/media/dxva_video_decode_accelerator.cc (right): https://chromiumcodereview.appspot.com/11369229/diff/31002/content/common/gpu/media/dxva_video_decode_accelerator.cc#newcode76 content/common/gpu/media/dxva_video_decode_accelerator.cc:76: // the batched queries to the ...
8 years, 1 month ago (2012-11-15 21:57:49 UTC) #14
apatrick_chromium
lgtm
8 years, 1 month ago (2012-11-15 22:02:09 UTC) #15
ananta
8 years, 1 month ago (2012-11-15 23:13:45 UTC) #16
https://chromiumcodereview.appspot.com/11369229/diff/31002/content/common/gpu...
File content/common/gpu/media/dxva_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/11369229/diff/31002/content/common/gpu...
content/common/gpu/media/dxva_video_decode_accelerator.cc:76: // the batched
queries to the driver.
On 2012/11/15 21:57:49, Ami Fischman wrote:
> s/driver/driver (and allow torn/corrupt frames to be rendered)/

Done.

https://chromiumcodereview.appspot.com/11369229/diff/31002/content/common/gpu...
content/common/gpu/media/dxva_video_decode_accelerator.cc:344: // Workaround is
to have a upper limit of 10 on the number of iterations we
On 2012/11/15 21:57:49, Ami Fischman wrote:
> s/a upper/an upper/

Done.

Powered by Google App Engine
This is Rietveld 408576698