Attempt to fix a crash in the DXVA decoder in the DXVAVideoDecodeAccelerator::ProcessPendingSamples…

Attempt to fix a crash in the DXVA decoder in the DXVAVideoDecodeAccelerator::ProcessPendingSamples function.

Based on the crash dump, the crash occurs after the call to the CopyOutputSampleDataToPictureBuffer function. This function
attempts to copy the decoded texture out to the target surface. Prior to doing that it initializes the video format converter
object, which essentially means setting some attributes on the object, setting its input and output media types.

If the initialization of the converter fails, then this results in a call to the RETURN_AND_NOTIFY_ON_FAILURE macro which internally
invalidates the decoder. The crash occurs on return from here as the iterator is now invalid.

Proposed fixes are as below:-.
1. Move the setting of the MF_XVP_PLAYBACK_MODE and MF_LOW_LATENCY attributes on the format converter to the initialization code.
   This should ensure that we fail early if these attributes cannot be set on the converter.

2. Set the minimum attributes on the media type in the converter. These include the MFMediaType_Video, MF_MT_SUBTYPE and MF_MT_FRAME_SIZE attributes.
   The rest are not needed.

3. Try to fallback to MFVideoFormat_RGB32 if we fail to find MFVideoFormat_ARGB32 as a supported output type in the converter.

If the above steps fail, then we have some CHECK's in the code which should help catch and analyze crashes in this code path.

BUG=495216
Committed: https://crrev.com/734cc3d830754f794d576db55f54f77c958315d7
Cr-Commit-Position: refs/heads/master@{#369882}

[ananta, 2016-01-15 03:29:23]

ananta@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[DaleCurtis, 2016-01-15 18:21:26]

dalecurtis@chromium.org changed reviewers:
+ sandersd@chromium.org
- dalecurtis@chromium.org

[DaleCurtis, 2016-01-15 18:21:26]

=>sandersd

[sandersd (OOO until July 31), 2016-01-15 19:25:02]

lgtm % nits.

https://codereview.chromium.org/1591603002/diff/1/content/common/gpu/media/dx...
File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right):

https://codereview.chromium.org/1591603002/diff/1/content/common/gpu/media/dx...
content/common/gpu/media/dxva_video_decode_accelerator_win.cc:2223:
CHECK(false);
Nit: Add an error message here?

https://codereview.chromium.org/1591603002/diff/1/content/common/gpu/media/dx...
content/common/gpu/media/dxva_video_decode_accelerator_win.cc:2296:
CHECK(false);
Should this just be returning false in case the fallback works, or is this
failure really much more serious?

[ananta, 2016-01-15 20:23:40]

ananta@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[ananta, 2016-01-15 20:23:41]

https://codereview.chromium.org/1591603002/diff/1/content/common/gpu/media/dx...
File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right):

https://codereview.chromium.org/1591603002/diff/1/content/common/gpu/media/dx...
content/common/gpu/media/dxva_video_decode_accelerator_win.cc:2223:
CHECK(false);
On 2016/01/15 19:25:02, sandersd wrote:
> Nit: Add an error message here?

Done.

https://codereview.chromium.org/1591603002/diff/1/content/common/gpu/media/dx...
content/common/gpu/media/dxva_video_decode_accelerator_win.cc:2296:
CHECK(false);
On 2016/01/15 19:25:02, sandersd wrote:
> Should this just be returning false in case the fallback works, or is this
> failure really much more serious?

If the fallback works then I think we are good. I tried with RGB and ARGB as the
output types. Could not see a visual difference in output quality.

[ananta, 2016-01-15 22:51:57]

The CQ bit was checked by ananta@chromium.org

[ananta, 2016-01-15 22:51:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
sandersd@chromium.org
Link to the patchset: https://codereview.chromium.org/1591603002/#ps20001
(title: "Address review comments")

[commit-bot: I haz the power, 2016-01-15 22:52:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1591603002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1591603002/20001

[commit-bot: I haz the power, 2016-01-15 23:50:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-15 23:51:54]

Description was changed from

==========
Attempt to fix a crash in the DXVA decoder in the
DXVAVideoDecodeAccelerator::ProcessPendingSamples function.

Based on the crash dump, the crash occurs after the call to the
CopyOutputSampleDataToPictureBuffer function. This function
attempts to copy the decoded texture out to the target surface. Prior to doing
that it initializes the video format converter
object, which essentially means setting some attributes on the object, setting
its input and output media types.

If the initialization of the converter fails, then this results in a call to the
RETURN_AND_NOTIFY_ON_FAILURE macro which internally
invalidates the decoder. The crash occurs on return from here as the iterator is
now invalid.

Proposed fixes are as below:-.
1. Move the setting of the MF_XVP_PLAYBACK_MODE and MF_LOW_LATENCY attributes on
the format converter to the initialization code.
   This should ensure that we fail early if these attributes cannot be set on
the converter.

2. Set the minimum attributes on the media type in the converter. These include
the MFMediaType_Video, MF_MT_SUBTYPE and MF_MT_FRAME_SIZE attributes.
   The rest are not needed.

3. Try to fallback to MFVideoFormat_RGB32 if we fail to find
MFVideoFormat_ARGB32 as a supported output type in the converter.

If the above steps fail, then we have some CHECK's in the code which should help
catch and analyze crashes in this code path.

BUG=495216
==========

to

==========
Attempt to fix a crash in the DXVA decoder in the
DXVAVideoDecodeAccelerator::ProcessPendingSamples function.

Based on the crash dump, the crash occurs after the call to the
CopyOutputSampleDataToPictureBuffer function. This function
attempts to copy the decoded texture out to the target surface. Prior to doing
that it initializes the video format converter
object, which essentially means setting some attributes on the object, setting
its input and output media types.

If the initialization of the converter fails, then this results in a call to the
RETURN_AND_NOTIFY_ON_FAILURE macro which internally
invalidates the decoder. The crash occurs on return from here as the iterator is
now invalid.

Proposed fixes are as below:-.
1. Move the setting of the MF_XVP_PLAYBACK_MODE and MF_LOW_LATENCY attributes on
the format converter to the initialization code.
   This should ensure that we fail early if these attributes cannot be set on
the converter.

2. Set the minimum attributes on the media type in the converter. These include
the MFMediaType_Video, MF_MT_SUBTYPE and MF_MT_FRAME_SIZE attributes.
   The rest are not needed.

3. Try to fallback to MFVideoFormat_RGB32 if we fail to find
MFVideoFormat_ARGB32 as a supported output type in the converter.

If the above steps fail, then we have some CHECK's in the code which should help
catch and analyze crashes in this code path.

BUG=495216

Committed: https://crrev.com/734cc3d830754f794d576db55f54f77c958315d7
Cr-Commit-Position: refs/heads/master@{#369882}
==========

[commit-bot: I haz the power, 2016-01-15 23:51:55]

Patchset 2 (id:??) landed as
https://crrev.com/734cc3d830754f794d576db55f54f77c958315d7
Cr-Commit-Position: refs/heads/master@{#369882}