Use texture ids passed from ARC as service ids in ArcGVDA

Use texture ids passed from ARC as service ids in ArcGVDA

The recent crrev.com/d3088b3826b4122fc534096da1f06b9383e9bdd2
reorganized how texture ids are managed in PictureBuffers, and resulted
in GL texture ids passed to ArcGVDA being used as client texture ids,
instead of service texture ids.

Also, crrev.com/83ae56bab030686ec47692edf2243dfcd680e7a7 uses GL texture
ids as both service and client ids. Make it pass an empty set of client
ids instead.

TEST=video playback on ARC, vdatest
BUG=b/32508278

Committed: https://crrev.com/4801e0b7872b2f36dd285e02b8fb7bbc804add6c
Cr-Commit-Position: refs/heads/master@{#429505}

[Pawel Osciak, 2016-11-01 07:30:22]

posciak@chromium.org changed reviewers:
+ watk@chromium.org, wuchengli@chromium.org

[Pawel Osciak, 2016-11-01 07:30:22]

ptal

[Pawel Osciak, 2016-11-01 07:30:41]

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

[commit-bot: I haz the power, 2016-11-01 07:30:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Pawel Osciak, 2016-11-01 08:04:02]

We also have a DCHECK at
https://cs.chromium.org/chromium/src/media/video/picture.cc?rcl=0&l=23, which
verifies that the counts of client and service texture ids passed are the same.
This makes sense if we have both and are using the constructor to pass both
sets, but if we are only passing service ids, we will fail the check.

My understanding however is that we shouldn't be using service ids as client
ids, hence the change to pass an empty vector for client ids instead. On the
other hand, we could have a constructor taking service ids only, but that's not
possible if we also have another constructor taking client ids only (not
mentioning that would be error prone).

So I'm leaning towards removing the DCHECK and passing empty client ids vector
as in this CL. Any thoughts please?

[commit-bot: I haz the power, 2016-11-01 08:16:08]

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

[commit-bot: I haz the power, 2016-11-01 08:16:09]

Dry run: This issue passed the CQ dry run.

[wuchengli, 2016-11-01 08:28:27]

I tested chrome tot. ARC playback was broken. When YouTube app played a video,
chrome crashed. After I applied this CL, ARC playback worked.

[Pawel Osciak, 2016-11-02 01:34:31]

posciak@chromium.org changed reviewers:
+ sandersd@chromium.org

[Pawel Osciak, 2016-11-02 01:34:31]

Removed DCHECK per above explanation.

+sandersd@ for OWNERS on media/video/picture.cc please. Thank you!

[Pawel Osciak, 2016-11-02 01:38:53]

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

[commit-bot: I haz the power, 2016-11-02 01:39:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[wuchengli, 2016-11-02 02:10:22]

https://codereview.chromium.org/2462393002/diff/20001/media/video/picture.cc
File media/video/picture.cc (left):

https://codereview.chromium.org/2462393002/diff/20001/media/video/picture.cc#...
media/video/picture.cc:23: DCHECK_EQ(client_texture_ids.size(),
service_texture_ids.size());
Pawel and I had an offline discussion. We decided it's better to keep the size
of client id and service id the same. So we'll pass a vector of 0 to indicate no
client ids.

[commit-bot: I haz the power, 2016-11-02 03:10:42]

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

[commit-bot: I haz the power, 2016-11-02 03:10:43]

Dry run: This issue passed the CQ dry run.

[Pawel Osciak, 2016-11-02 09:18:55]

Patchset #3 (id:40001) has been deleted

[Pawel Osciak, 2016-11-02 09:21:49]

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

[commit-bot: I haz the power, 2016-11-02 09:21:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Pawel Osciak, 2016-11-02 09:23:47]

Since we'd have the mailboxes vector empty anyway, I think now that possibly the
most intuitive would be to have empty vectors for id types that are not
available, and only verify that if we do provide any ids at all, then their
counts are the same.

Modified accordingly. PTAL.

[wuchengli, 2016-11-02 10:03:12]

lgtm

[commit-bot: I haz the power, 2016-11-02 11:02:33]

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

[commit-bot: I haz the power, 2016-11-02 11:02:33]

Dry run: This issue passed the CQ dry run.

[watk, 2016-11-02 18:27:52]

I think I agree that an empty vector is the least error prone way to go. Sorry
for the breakage. Seems like some more CQ coverage would help here.

lgtm

[watk, 2016-11-02 18:53:00]

On 2016/11/02 18:27:52, watk wrote:
> I think I agree that an empty vector is the least error prone way to go. Sorry
> for the breakage. Seems like some more CQ coverage would help here.
> 
> lgtm

Actually. This will probably make us hit a DCHECK in DXVA
https://cs.chromium.org/chromium/src/media/gpu/dxva_video_decode_accelerator_...

Btw for future reference, is it possible for me to test the ChromeOS VDAs on
linux?

[Pawel Osciak, 2016-11-02 23:59:52]

Patchset #4 (id:80001) has been deleted

[Pawel Osciak, 2016-11-03 00:01:40]

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

[Pawel Osciak, 2016-11-03 00:02:31]

posciak@chromium.org changed reviewers:
+ jbauman@chromium.org

[Pawel Osciak, 2016-11-03 00:02:32]

Fixed up DXVA.

jbauman@: may I ask for a review of DXVA changes please? Thanks.

[commit-bot: I haz the power, 2016-11-03 00:02:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sandersd (OOO until July 31), 2016-11-03 00:18:39]

DXVA lgtm.

[Pawel Osciak, 2016-11-03 00:26:23]

The CQ bit was unchecked by posciak@chromium.org

[Pawel Osciak, 2016-11-03 00:26:27]

The CQ bit was checked by posciak@chromium.org

[Pawel Osciak, 2016-11-03 00:26:27]

The patchset sent to the CQ was uploaded after l-g-t-m from watk@chromium.org,
wuchengli@chromium.org
Link to the patchset: https://codereview.chromium.org/2462393002/#ps100001
(title: "Fixup DXVA")

[commit-bot: I haz the power, 2016-11-03 00:26:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-03 01:43:45]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-11-03 01:45:30]

Description was changed from

==========
Use texture ids passed from ARC as service ids in ArcGVDA

The recent crrev.com/d3088b3826b4122fc534096da1f06b9383e9bdd2
reorganized how texture ids are managed in PictureBuffers, and resulted
in GL texture ids passed to ArcGVDA being used as client texture ids,
instead of service texture ids.

Also, crrev.com/83ae56bab030686ec47692edf2243dfcd680e7a7 uses GL texture
ids as both service and client ids. Make it pass an empty set of client
ids instead.

TEST=video playback on ARC, vdatest
BUG=b/32508278
==========

to

==========
Use texture ids passed from ARC as service ids in ArcGVDA

The recent crrev.com/d3088b3826b4122fc534096da1f06b9383e9bdd2
reorganized how texture ids are managed in PictureBuffers, and resulted
in GL texture ids passed to ArcGVDA being used as client texture ids,
instead of service texture ids.

Also, crrev.com/83ae56bab030686ec47692edf2243dfcd680e7a7 uses GL texture
ids as both service and client ids. Make it pass an empty set of client
ids instead.

TEST=video playback on ARC, vdatest
BUG=b/32508278

Committed: https://crrev.com/4801e0b7872b2f36dd285e02b8fb7bbc804add6c
Cr-Commit-Position: refs/heads/master@{#429505}
==========

[commit-bot: I haz the power, 2016-11-03 01:45:31]

Patchset 4 (id:??) landed as
https://crrev.com/4801e0b7872b2f36dd285e02b8fb7bbc804add6c
Cr-Commit-Position: refs/heads/master@{#429505}