Chromecast media pipeline: handle CENC "full sample encryption" case.

Chromecast media pipeline: handle CENC "full sample encryption" case.

In this case, subsample information is allowed to be elided, but
hardware backends typically translate subsample information into
hardware APIs. To more cleanly map across this, translate this case
into one fully-encrypted "subsample" while transferring DecryptConfigs.

R=lcwu@chromium.org,damienv@chromium.org
BUG=None

Committed: https://crrev.com/57e4c85b678e003d8790fed9ada30496783b4e6c
Cr-Commit-Position: refs/heads/master@{#300123}

[damienv1, 2014-10-17 15:43:48]

I would not change DecryptConfigMarshaller but just DecoderBufferMarshaller
which would adapt the DecryptConfig when needed.

1- You don't need to pass any extra parameter
2- When the corner case is removed, it will be easy to remove it from
DecoderBufferMarshaller.

[damienv1, 2014-10-17 15:46:06]

https://codereview.chromium.org/659343003/diff/1/chromecast/media/cma/ipc_str...
File chromecast/media/cma/ipc_streamer/decoder_buffer_base_marshaller.cc
(right):

https://codereview.chromium.org/659343003/diff/1/chromecast/media/cma/ipc_str...
chromecast/media/cma/ipc_streamer/decoder_buffer_base_marshaller.cc:145: if
(has_decrypt_config) {
Note: I was thinking about sth like:

::media::DecryptConifg* decrypt_config = buffer->decrypt_config();

// Handle the corner case:  
scoped_ptr<::media::DecryptConifg> equivalent_decrypt_config;
if (decrypt_config->subsamples().empty()) {
  std::vector<media::SubsampleEntry> subsamples(1);
  subsamples[0].clear_bytes = 0;
  subsamples[0].cypher_bytes = buffer->data_size();
  decrypt_config_alternate.reset(new ::media::DecryptConfig(key, iv,
subsamples);
}

[gunsch, 2014-10-17 16:04:03]

https://codereview.chromium.org/659343003/diff/1/chromecast/media/cma/ipc_str...
File chromecast/media/cma/ipc_streamer/decoder_buffer_base_marshaller.cc
(right):

https://codereview.chromium.org/659343003/diff/1/chromecast/media/cma/ipc_str...
chromecast/media/cma/ipc_streamer/decoder_buffer_base_marshaller.cc:145: if
(has_decrypt_config) {
On 2014/10/17 15:46:05, damienv1 wrote:
> Note: I was thinking about sth like:
> 
> ::media::DecryptConifg* decrypt_config = buffer->decrypt_config();
> 
> // Handle the corner case:  
> scoped_ptr<::media::DecryptConifg> equivalent_decrypt_config;
> if (decrypt_config->subsamples().empty()) {
> &nbsp;&nbsp;std::vector<media::SubsampleEntry> subsamples(1);
> &nbsp; subsamples[0].clear_bytes = 0;
> &nbsp; subsamples[0].cypher_bytes = buffer->data_size();
> &nbsp; decrypt_config_alternate.reset(new ::media::DecryptConfig(key, iv,
> subsamples);
> }

heh, I had something exactly like this earlier and rewrote it because I thought
this approach was preferable :)

[damienv1, 2014-10-17 16:20:22]

lgtm % nits

https://codereview.chromium.org/659343003/diff/20001/chromecast/media/cma/ipc...
File chromecast/media/cma/ipc_streamer/decoder_buffer_base_marshaller.cc
(right):

https://codereview.chromium.org/659343003/diff/20001/chromecast/media/cma/ipc...
chromecast/media/cma/ipc_streamer/decoder_buffer_base_marshaller.cc:147: //
ISO/IEC 23001-7:2012 9.5 by declaring one "subsample" with
nit: Not sure you should refer to the spec here. DecryptConfig is the Chrome
internal representation and does not have to match any spec (even though it is
currently re-using the same convention as the CENC spec). I would rather simply
refer to the DecryptConfig header file.

https://codereview.chromium.org/659343003/diff/20001/chromecast/media/cma/ipc...
chromecast/media/cma/ipc_streamer/decoder_buffer_base_marshaller.cc:159: else
nit: I think you also need curly braces for the "else" (multi line if/else
statement).

[gunsch, 2014-10-17 16:22:52]

https://codereview.chromium.org/659343003/diff/20001/chromecast/media/cma/ipc...
File chromecast/media/cma/ipc_streamer/decoder_buffer_base_marshaller.cc
(right):

https://codereview.chromium.org/659343003/diff/20001/chromecast/media/cma/ipc...
chromecast/media/cma/ipc_streamer/decoder_buffer_base_marshaller.cc:147: //
ISO/IEC 23001-7:2012 9.5 by declaring one "subsample" with
On 2014/10/17 16:20:22, damienv1 wrote:
> nit: Not sure you should refer to the spec here. DecryptConfig is the Chrome
> internal representation and does not have to match any spec (even though it is
> currently re-using the same convention as the CENC spec). I would rather
simply
> refer to the DecryptConfig header file.

Done.

https://codereview.chromium.org/659343003/diff/20001/chromecast/media/cma/ipc...
chromecast/media/cma/ipc_streamer/decoder_buffer_base_marshaller.cc:159: else
On 2014/10/17 16:20:22, damienv1 wrote:
> nit: I think you also need curly braces for the "else" (multi line if/else
> statement).

Done.

[lcwu1, 2014-10-17 16:26:26]

LGTM

https://codereview.chromium.org/659343003/diff/20001/chromecast/media/cma/ipc...
File chromecast/media/cma/ipc_streamer/decoder_buffer_base_marshaller.cc
(right):

https://codereview.chromium.org/659343003/diff/20001/chromecast/media/cma/ipc...
chromecast/media/cma/ipc_streamer/decoder_buffer_base_marshaller.cc:159: else
Also else should be in the same line as the previous '}'.

[damienv1, 2014-10-17 16:29:51]

lgtm

[gunsch, 2014-10-17 16:30:47]

The CQ bit was checked by gunsch@chromium.org

[commit-bot: I haz the power, 2014-10-17 16:31:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/659343003/80001

[commit-bot: I haz the power, 2014-10-17 17:31:54]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2014-10-17 17:33:01]

Patchset 5 (id:??) landed as
https://crrev.com/57e4c85b678e003d8790fed9ada30496783b4e6c
Cr-Commit-Position: refs/heads/master@{#300123}