ppapi: Support up to 32 subsamples in PP_EncryptedBlockInfo.

ppapi: Support up to 32 subsamples in PP_EncryptedBlockInfo.

The previous value was 16 which worked for most cases. But recently we saw
reports that there are cases that we could have 17 subsmples. Even thougt this
is very rare, but it makes sense to update our implementation to support that
case.

Our current implementation [1-2] supports this new size without requiring any
changes.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/media/cdm/ppapi/cdm_adapter.cc&rcl=1445081078&l=79
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/pepper/content_decryptor_delegate.cc&l=128

BUG=543478
Committed: https://crrev.com/e9a0ee342d8c663a6787d1a42ab29d3dbf265395
Cr-Commit-Position: refs/heads/master@{#355157}

[xhwang, 2015-10-19 20:47:26]

xhwang@chromium.org changed reviewers:
+ bbudge@chromium.org

[xhwang, 2015-10-19 20:47:27]

I only updated the two *.idl files and generator.py changed a bunch other files
for me. bbudge@: Does this look right to you?

ddorwin: PTAL from media/eme's perspective.
bbudge: PTAL from ppapi's perspective.

https://chromiumcodereview.appspot.com/1412323002/diff/1/ppapi/api/private/pp...
File ppapi/api/private/pp_content_decryptor.idl (right):

https://chromiumcodereview.appspot.com/1412323002/diff/1/ppapi/api/private/pp...
ppapi/api/private/pp_content_decryptor.idl:122:
PP_DecryptSubsampleDescription[32] subsamples;
This is the real change in this CL :)

https://chromiumcodereview.appspot.com/1412323002/diff/1/ppapi/api/private/pp...
File ppapi/api/private/ppp_content_decryptor_private.idl (right):

https://chromiumcodereview.appspot.com/1412323002/diff/1/ppapi/api/private/pp...
ppapi/api/private/ppp_content_decryptor_private.idl:12: M48 = 0.16
There's no direct change in PPP_ContentDecryptor_Private interface, but
PPP_ContentDecryptor_Private interface uses PP_EncryptedBlockInfo which has been
changed. So I think we should still bump the label here. Is this correct?

[bbudge, 2015-10-20 17:38:15]

I'm not sure why the thunk files are updated. Perhaps the default formatting
changed. It's better if your CL doesn't include these changes but I'm not sure
how to get around this. The presubmit test may fail if you revert the change
(git checkout origin/master ppapi/thunk.)

https://codereview.chromium.org/1412323002/diff/1/ppapi/api/private/pp_conten...
File ppapi/api/private/pp_content_decryptor.idl (right):

https://codereview.chromium.org/1412323002/diff/1/ppapi/api/private/pp_conten...
ppapi/api/private/pp_content_decryptor.idl:122:
PP_DecryptSubsampleDescription[32] subsamples;
On 2015/10/19 20:47:26, xhwang wrote:
> This is the real change in this CL :)
Usually we version the struct. See ppb_video_decoder.idl and pp_codec.idl for
examples.

https://codereview.chromium.org/1412323002/diff/1/ppapi/api/private/ppp_conte...
File ppapi/api/private/ppp_content_decryptor_private.idl (right):

https://codereview.chromium.org/1412323002/diff/1/ppapi/api/private/ppp_conte...
ppapi/api/private/ppp_content_decryptor_private.idl:12: M48 = 0.16
On 2015/10/19 20:47:26, xhwang wrote:
> There's no direct change in PPP_ContentDecryptor_Private interface, but
> PPP_ContentDecryptor_Private interface uses PP_EncryptedBlockInfo which has
been
> changed. So I think we should still bump the label here. Is this correct?

I'm not that familiar with content decryption, but if plugins that use it aren't
released in sync with Chrome, then you need to keep the old version and support
the older struct. See my comment on PP_EncryptedBlockInfo.

[xhwang, 2015-10-20 18:30:55]

I removed changes in thunk and presubmit isn't complaining :)

Please see my response below.

https://codereview.chromium.org/1412323002/diff/1/ppapi/api/private/pp_conten...
File ppapi/api/private/pp_content_decryptor.idl (right):

https://codereview.chromium.org/1412323002/diff/1/ppapi/api/private/pp_conten...
ppapi/api/private/pp_content_decryptor.idl:122:
PP_DecryptSubsampleDescription[32] subsamples;
On 2015/10/20 17:38:15, bbudge wrote:
> On 2015/10/19 20:47:26, xhwang wrote:
> > This is the real change in this CL :)
> Usually we version the struct. See ppb_video_decoder.idl and pp_codec.idl for
> examples.

(Also for your question below.)

We always ship the plugin together (in-sync) with Chrome, so there are no
compatibility issues. In that case, I guess we don't need to keep the old
structure and use versioning here?

https://codereview.chromium.org/1412323002/diff/1/ppapi/api/private/ppp_conte...
File ppapi/api/private/ppp_content_decryptor_private.idl (right):

https://codereview.chromium.org/1412323002/diff/1/ppapi/api/private/ppp_conte...
ppapi/api/private/ppp_content_decryptor_private.idl:12: M48 = 0.16
On 2015/10/20 17:38:15, bbudge wrote:
> On 2015/10/19 20:47:26, xhwang wrote:
> > There's no direct change in PPP_ContentDecryptor_Private interface, but
> > PPP_ContentDecryptor_Private interface uses PP_EncryptedBlockInfo which has
> been
> > changed. So I think we should still bump the label here. Is this correct?
> 
> I'm not that familiar with content decryption, but if plugins that use it
aren't
> released in sync with Chrome, then you need to keep the old version and
support
> the older struct. See my comment on PP_EncryptedBlockInfo.

See above.

[bbudge, 2015-10-20 19:33:39]

LGTM

https://codereview.chromium.org/1412323002/diff/20001/ppapi/api/private/pp_co...
File ppapi/api/private/pp_content_decryptor.idl (right):

https://codereview.chromium.org/1412323002/diff/20001/ppapi/api/private/pp_co...
ppapi/api/private/pp_content_decryptor.idl:117: * TODO(xhwang): We need to have
a fixed size of |subsamples| here. Choose 32
It's not clear what the TODO is here.

[xhwang, 2015-10-20 19:51:41]

comments addressed

[xhwang, 2015-10-20 19:52:38]

https://chromiumcodereview.appspot.com/1412323002/diff/20001/ppapi/api/privat...
File ppapi/api/private/pp_content_decryptor.idl (right):

https://chromiumcodereview.appspot.com/1412323002/diff/20001/ppapi/api/privat...
ppapi/api/private/pp_content_decryptor.idl:117: * TODO(xhwang): We need to have
a fixed size of |subsamples| here. Choose 32
On 2015/10/20 19:33:39, bbudge wrote:
> It's not clear what the TODO is here.

Agreed. Remove TODO here and above.

[xhwang, 2015-10-20 20:33:02]

The CQ bit was checked by xhwang@chromium.org

[xhwang, 2015-10-20 20:33:02]

The patchset sent to the CQ was uploaded after l-g-t-m from bbudge@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1412323002/#ps40001
(title: "comments addressed")

[commit-bot: I haz the power, 2015-10-20 20:35:19]

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

[commit-bot: I haz the power, 2015-10-20 21:35:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-20 21:36:58]

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