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

Issue 1235373003: Fix a crasher in the GPU process caused by the DXVA code attempting to create an instance of the Vi… (Closed)

Created:
5 years, 5 months ago by ananta
Modified:
5 years, 5 months ago
Reviewers:
DaleCurtis
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, wjia+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix a crasher in the GPU process caused by the DXVA code attempting to create an instance of the Video processor MFT The DXVA decoder object on Windows 8+ uses DX11 if available and in this mode it uses the VideoProcessor MFT for colorspace conversion. This object exposes the IMFTransform interface and can be created like a traditional COM object via the CoCreateInstance API. However this API expects CoInitialize to be called which is not the case on the main GPU thread and fails with an error indicating that the COM apartment is not initialized. It is not clear as to why it is not failing in all cases. Proposed fix is to use DllGetClassObject and IClassFactory::CreateInstance to create the object directly and avoid using CoCreateInstance. For this purpose added a helper function CreateCOMObjectFromDll in the dxva code. BUG=493894 R=dalecurtis Committed: https://crrev.com/f74c38420401d72fc8fdae2b8d03aece1279f7e7 Cr-Commit-Position: refs/heads/master@{#338975}

Patch Set 1 #

Total comments: 8

Patch Set 2 : Address review comments #

Total comments: 2

Patch Set 3 : Fix indent #

Unified diffs Side-by-side diffs Delta from patch set Stats (+48 lines, -37 lines) Patch
M content/common/gpu/media/dxva_video_decode_accelerator.cc View 1 2 8 chunks +48 lines, -37 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 12 (1 generated)
ananta
5 years, 5 months ago (2015-07-15 22:57:44 UTC) #1
DaleCurtis
I don't know enough to say if that's the right dll to use, but if ...
5 years, 5 months ago (2015-07-15 23:11:56 UTC) #2
ananta
https://codereview.chromium.org/1235373003/diff/1/content/common/gpu/media/dxva_video_decode_accelerator.cc File content/common/gpu/media/dxva_video_decode_accelerator.cc (right): https://codereview.chromium.org/1235373003/diff/1/content/common/gpu/media/dxva_video_decode_accelerator.cc#newcode254 content/common/gpu/media/dxva_video_decode_accelerator.cc:254: typedef HRESULT(WINAPI * GetClassObject)( On 2015/07/15 23:11:56, DaleCurtis wrote: ...
5 years, 5 months ago (2015-07-15 23:15:01 UTC) #3
ananta
https://codereview.chromium.org/1235373003/diff/1/content/common/gpu/media/dxva_video_decode_accelerator.cc File content/common/gpu/media/dxva_video_decode_accelerator.cc (right): https://codereview.chromium.org/1235373003/diff/1/content/common/gpu/media/dxva_video_decode_accelerator.cc#newcode254 content/common/gpu/media/dxva_video_decode_accelerator.cc:254: typedef HRESULT(WINAPI * GetClassObject)( On 2015/07/15 23:11:56, DaleCurtis wrote: ...
5 years, 5 months ago (2015-07-15 23:23:25 UTC) #4
DaleCurtis
https://codereview.chromium.org/1235373003/diff/20001/content/common/gpu/media/dxva_video_decode_accelerator.cc File content/common/gpu/media/dxva_video_decode_accelerator.cc (right): https://codereview.chromium.org/1235373003/diff/20001/content/common/gpu/media/dxva_video_decode_accelerator.cc#newcode255 content/common/gpu/media/dxva_video_decode_accelerator.cc:255: const CLSID &clsid, const IID& iid, void **object); & ...
5 years, 5 months ago (2015-07-16 00:22:41 UTC) #5
ananta
https://codereview.chromium.org/1235373003/diff/20001/content/common/gpu/media/dxva_video_decode_accelerator.cc File content/common/gpu/media/dxva_video_decode_accelerator.cc (right): https://codereview.chromium.org/1235373003/diff/20001/content/common/gpu/media/dxva_video_decode_accelerator.cc#newcode255 content/common/gpu/media/dxva_video_decode_accelerator.cc:255: const CLSID &clsid, const IID& iid, void **object); On ...
5 years, 5 months ago (2015-07-16 00:24:13 UTC) #6
DaleCurtis
lgtm
5 years, 5 months ago (2015-07-16 00:30:29 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1235373003/40001
5 years, 5 months ago (2015-07-16 01:38:52 UTC) #9
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 5 months ago (2015-07-16 02:41:29 UTC) #10
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/f74c38420401d72fc8fdae2b8d03aece1279f7e7 Cr-Commit-Position: refs/heads/master@{#338975}
5 years, 5 months ago (2015-07-16 02:42:36 UTC) #11
brucedawson
5 years, 5 months ago (2015-07-17 01:08:16 UTC) #12
Message was sent while issue was closed.
Small bug hidden in the macros:

https://code.google.com/p/chromium/issues/detail?id=511044

Powered by Google App Engine
This is Rietveld 408576698