media: Properly tell whether the buffer is encrypted

media: Properly tell whether the buffer is encrypted

We need to call MediaCodec method queueSecureInputBuffer()
instead of queueInputBuffer() only if both the key and the
initialization vector are not empty.

BUG=605341
Committed: https://crrev.com/100cc77fa6654a0198561e6621f6bf10fa9adc3f
Cr-Commit-Position: refs/heads/master@{#389229}

[Tima Vaisburd, 2016-04-22 17:37:18]

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

[Tima Vaisburd, 2016-04-22 17:37:19]

PTAL

[xhwang, 2016-04-22 18:05:44]

Does it make sense to put is_encrypted() in DecoderBuffer directly?

[Tima Vaisburd, 2016-04-22 18:14:56]

On 2016/04/22 18:05:44, xhwang wrote:
> Does it make sense to put is_encrypted() in DecoderBuffer directly?

I'm not sure, please notice that I need DecryptConfig pointer anyway down below.
And I would implement DecoderBuffer::is_encrypted() as
 | return decoder_config_ && decoder_config_->is_encrypted(); | so we would have
DecoderConfig::is_encrypted() anyway.

Maybe you'd like a better name for the method, because the buffer can be
encrypted,
and configuration just tells about that. I couldn't come up.

[Tima Vaisburd, 2016-04-22 19:08:27]

On 2016/04/22 18:14:56, Tima Vaisburd wrote:
> On 2016/04/22 18:05:44, xhwang wrote:
> > Does it make sense to put is_encrypted() in DecoderBuffer directly?
> 
> I'm not sure, please notice that I need DecryptConfig pointer anyway down
below.
> And I would implement DecoderBuffer::is_encrypted() as
>  | return decoder_config_ && decoder_config_->is_encrypted(); | so we would
have
> DecoderConfig::is_encrypted() anyway.
> 
> Maybe you'd like a better name for the method, because the buffer can be
> encrypted,
> and configuration just tells about that. I couldn't come up.

WDYT?

[xhwang, 2016-04-22 19:18:54]

On 2016/04/22 19:08:27, Tima Vaisburd wrote:
> On 2016/04/22 18:14:56, Tima Vaisburd wrote:
> > On 2016/04/22 18:05:44, xhwang wrote:
> > > Does it make sense to put is_encrypted() in DecoderBuffer directly?
> > 
> > I'm not sure, please notice that I need DecryptConfig pointer anyway down
> below.
> > And I would implement DecoderBuffer::is_encrypted() as
> >  | return decoder_config_ && decoder_config_->is_encrypted(); | so we would
> have
> > DecoderConfig::is_encrypted() anyway.
> > 
> > Maybe you'd like a better name for the method, because the buffer can be
> > encrypted,
> > and configuration just tells about that. I couldn't come up.
> 
> WDYT?

Ultimately we shouldn't rely on empty IV or key ID to indicate that the buffer
is not encrypted. Then we can see what's the best way to represent this. But
that deserves another CL.

lgtm!

[xhwang, 2016-04-22 19:18:54]

On 2016/04/22 19:08:27, Tima Vaisburd wrote:
> On 2016/04/22 18:14:56, Tima Vaisburd wrote:
> > On 2016/04/22 18:05:44, xhwang wrote:
> > > Does it make sense to put is_encrypted() in DecoderBuffer directly?
> > 
> > I'm not sure, please notice that I need DecryptConfig pointer anyway down
> below.
> > And I would implement DecoderBuffer::is_encrypted() as
> >  | return decoder_config_ && decoder_config_->is_encrypted(); | so we would
> have
> > DecoderConfig::is_encrypted() anyway.
> > 
> > Maybe you'd like a better name for the method, because the buffer can be
> > encrypted,
> > and configuration just tells about that. I couldn't come up.
> 
> WDYT?

Ultimately we shouldn't rely on empty IV or key ID to indicate that the buffer
is not encrypted. Then we can see what's the best way to represent this. But
that deserves another CL.

lgtm!

[Tima Vaisburd, 2016-04-22 19:20:28]

The CQ bit was checked by timav@chromium.org

[commit-bot: I haz the power, 2016-04-22 19:20:58]

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

[commit-bot: I haz the power, 2016-04-22 20:41:32]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-04-22 20:42:53]

Description was changed from

==========
media: Properly tell whether the buffer is encrypted

We need to call MediaCodec method queueSecureInputBuffer()
instead of queueInputBuffer() only if both the key and the
initialization vector are not empty.

BUG=605341
==========

to

==========
media: Properly tell whether the buffer is encrypted

We need to call MediaCodec method queueSecureInputBuffer()
instead of queueInputBuffer() only if both the key and the
initialization vector are not empty.

BUG=605341

Committed: https://crrev.com/100cc77fa6654a0198561e6621f6bf10fa9adc3f
Cr-Commit-Position: refs/heads/master@{#389229}
==========

[commit-bot: I haz the power, 2016-04-22 20:42:54]

Patchset 1 (id:??) landed as
https://crrev.com/100cc77fa6654a0198561e6621f6bf10fa9adc3f
Cr-Commit-Position: refs/heads/master@{#389229}