Moving the validation of bitstream_buffer into VDA implementations.

Moving the validation of bitstream_buffer into VDA implementations.

Also remove the validation on bitstream_buffer.handle() since there
will be errors when trying to map the handle.

BUG=None
Test=Play YT on ChromeOS Device.

Committed: https://crrev.com/df99f6ba8f296d3cb35544997aa120274c064e88
Cr-Commit-Position: refs/heads/master@{#377234}

[Owen Lin, 2016-02-18 05:03:00]

The CQ bit was checked by owenlin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-18 05:03:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1706023003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1706023003/1

[commit-bot: I haz the power, 2016-02-18 05:24:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-18 05:24:16]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Owen Lin, 2016-02-18 06:11:59]

Patchset #1 (id:1) has been deleted

[Owen Lin, 2016-02-18 06:12:50]

owenlin@chromium.org changed reviewers:
+ kcwu@chromium.org, posciak@chromium.org

[Owen Lin, 2016-02-18 06:12:50]

PTAL. Thanks.

[kcwu, 2016-02-18 06:57:02]

lgtm

[Pawel Osciak, 2016-02-19 07:48:44]

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
File content/common/gpu/media/android_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
content/common/gpu/media/android_video_decode_accelerator.cc:621: // We use id =
-1 as a special bitstream_buffer internally, which indicating
This feels like an opportunity to remove the check at l.637:

Decode() {
  if (id > 0 && size != 0) {
    DecodeBuffer(bitstream_buffer);
    return;
  }

  if (handle valid) {
    CloseHandle();
  }

  if (id < 0) {
    POST_ERROR
  } else {
    NEOBB()
  }
}

DecodeBuffer() {
   pending.push();
   DoIOTask();
}

and call DecodeBuffer from Flush() ?

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
content/common/gpu/media/android_video_decode_accelerator.cc:641:
weak_this_factory_.GetWeakPtr(), bitstream_buffer.id()));
We need to CloseHandle here as well, but please see above.

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
content/common/gpu/media/android_video_decode_accelerator.cc:832:
weak_this_factory_.GetWeakPtr(), bitstream_buffer_id));
Here we should also be closing the handle? Perhaps pending_bitstream_buffers
should just store SharedMemory to simplify things.

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right):

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
content/common/gpu/media/dxva_video_decode_accelerator_win.cc:949: 
Perhaps we could just create SharedMemory here and pass that with size to
CreateSampleFromInputBuffer() ?

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
File content/common/gpu/media/vaapi_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
content/common/gpu/media/vaapi_video_decode_accelerator.cc:676:
RETURN_AND_NOTIFY_ON_FAILURE(
I think we need to close if < 0 and valid?

[Owen Lin, 2016-02-23 08:19:55]

Patchset #2 (id:40001) has been deleted

[Owen Lin, 2016-02-23 09:06:38]

PTAL. Thanks.

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
File content/common/gpu/media/android_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
content/common/gpu/media/android_video_decode_accelerator.cc:621: // We use id =
-1 as a special bitstream_buffer internally, which indicating
On 2016/02/19 07:48:44, Pawel Osciak wrote:
> This feels like an opportunity to remove the check at l.637:
> 
> Decode() {
>   if (id > 0 && size != 0) {
>     DecodeBuffer(bitstream_buffer);
>     return;
>   }
> 
>   if (handle valid) {
>     CloseHandle();
>   }
> 
>   if (id < 0) {
>     POST_ERROR
>   } else {
>     NEOBB()
>   }
> }
> 
> DecodeBuffer() {
>    pending.push();
>    DoIOTask();
> }
> 
> and call DecodeBuffer from Flush() ?

Done.

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
content/common/gpu/media/android_video_decode_accelerator.cc:832:
weak_this_factory_.GetWeakPtr(), bitstream_buffer_id));
On 2016/02/19 07:48:44, Pawel Osciak wrote:
> Here we should also be closing the handle? Perhaps pending_bitstream_buffers
> should just store SharedMemory to simplify things.

Sounds good. But let me prepare the change after the SharedMemoryRegion is in.

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
File content/common/gpu/media/dxva_video_decode_accelerator_win.cc (right):

https://chromiumcodereview.appspot.com/1706023003/diff/20001/content/common/g...
content/common/gpu/media/dxva_video_decode_accelerator_win.cc:949: 
On 2016/02/19 07:48:44, Pawel Osciak wrote:
> Perhaps we could just create SharedMemory here and pass that with size to
> CreateSampleFromInputBuffer() ?

Done.

[Pawel Osciak, 2016-02-24 01:16:25]

lgtm

[Owen Lin, 2016-02-24 01:56:23]

Patchset #3 (id:80001) has been deleted

[Owen Lin, 2016-02-24 05:14:46]

The CQ bit was checked by owenlin@chromium.org

[Owen Lin, 2016-02-24 05:14:46]

The patchset sent to the CQ was uploaded after l-g-t-m from kcwu@chromium.org,
posciak@chromium.org
Link to the patchset: https://codereview.chromium.org/1706023003/#ps100001
(title: "fix compiling errors on windows")

[commit-bot: I haz the power, 2016-02-24 05:15:41]

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

[commit-bot: I haz the power, 2016-02-24 06:31:18]

Committed patchset #3 (id:100001)

[commit-bot: I haz the power, 2016-02-24 06:32:30]

Description was changed from

==========
Moving the validation of bitstream_buffer into VDA implementations.

Also remove the validation on bitstream_buffer.handle() since there
will be errors when trying to map the handle.

BUG=None
Test=Play YT on ChromeOS Device.
==========

to

==========
Moving the validation of bitstream_buffer into VDA implementations.

Also remove the validation on bitstream_buffer.handle() since there
will be errors when trying to map the handle.

BUG=None
Test=Play YT on ChromeOS Device.

Committed: https://crrev.com/df99f6ba8f296d3cb35544997aa120274c064e88
Cr-Commit-Position: refs/heads/master@{#377234}
==========

[commit-bot: I haz the power, 2016-02-24 06:32:31]

Patchset 3 (id:??) landed as
https://crrev.com/df99f6ba8f296d3cb35544997aa120274c064e88
Cr-Commit-Position: refs/heads/master@{#377234}