MediaCodecBridge: remove CodecInfo.mIsSecureDecoderSupported since it's unused & crashy.

MediaCodecBridge: remove CodecInfo.mIsSecureDecoderSupported since it's unused & crashy.

BUG=314868
R=wjia@chromium.org, xhwang@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=234675

[Ami GONE FROM CHROMIUM, 2013-11-11 19:13:35]

@xhwang: please review & confirm this is ok for EME.
@wjia: please review & confirm this fixes the GN issue.

[wjia(left Chromium), 2013-11-11 19:48:22]

Confirmed that this patch resolved crash problem. LGTM, with one question (not
related to this patch though).

https://codereview.chromium.org/68923003/diff/1/media/base/android/java/src/o...
File media/base/android/java/src/org/chromium/media/MediaCodecBridge.java
(right):

https://codereview.chromium.org/68923003/diff/1/media/base/android/java/src/o...
media/base/android/java/src/org/chromium/media/MediaCodecBridge.java:153: if
(!CodecInfoMap.containsKey(supportedTypes[j])) {
OOC, what if two codec's support same type? Based on the code, the second one is
ignore. But what if the second one is more efficient or has special capability,
e.g., faster but consuming more power?

[Ami GONE FROM CHROMIUM, 2013-11-11 19:51:49]

Thanks Wei.

https://codereview.chromium.org/68923003/diff/1/media/base/android/java/src/o...
File media/base/android/java/src/org/chromium/media/MediaCodecBridge.java
(right):

https://codereview.chromium.org/68923003/diff/1/media/base/android/java/src/o...
media/base/android/java/src/org/chromium/media/MediaCodecBridge.java:153: if
(!CodecInfoMap.containsKey(supportedTypes[j])) {
On 2013/11/11 19:48:22, wjia wrote:
> OOC, what if two codec's support same type? Based on the code, the second one
is
> ignore. But what if the second one is more efficient or has special
capability,
> e.g., faster but consuming more power?

The idea is that the android platform presents codecs from most-preferred to
least-preferred, so we take the first one that works.

This whole shebang can be simplified greatly now that we know the secure
business is unnecessary.  I kept this CL to a minimum for m32-mergeability.

[xhwang, 2013-11-11 20:06:00]

lgtm, can we add comments somewhere to say that we don't include .secure codecs
because they are crashing. Just to remind people the existence of those .secure
codecs.

[Ami GONE FROM CHROMIUM, 2013-11-11 20:45:55]

On Mon, Nov 11, 2013 at 12:06 PM, <xhwang@chromium.org> wrote:

> lgtm, can we add comments somewhere to say that we don't include .secure
> codecs
> because they are crashing. Just to remind people the existence of those
> .secure
> codecs.
>

I would rather everyone forget about these .secure codecs ;)
They are undocumented and apparently completely broken.  Why would you want
to spread the lore of them?


>
> https://codereview.chromium.org/68923003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[xhwang, 2013-11-11 20:50:40]

On 2013/11/11 20:45:55, Ami Fischman wrote:
> On Mon, Nov 11, 2013 at 12:06 PM, <mailto:xhwang@chromium.org> wrote:
> 
> > lgtm, can we add comments somewhere to say that we don't include .secure
> > codecs
> > because they are crashing. Just to remind people the existence of those
> > .secure
> > codecs.
> >
> 
> I would rather everyone forget about these .secure codecs ;)
> They are undocumented and apparently completely broken.  Why would you want
> to spread the lore of them?

Sure. Unfortunately our work depends on them :(

> 
> 
> >
> > https://codereview.chromium.org/68923003/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2013-11-11 21:04:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fischman@chromium.org/68923003/1

[Ami GONE FROM CHROMIUM, 2013-11-11 21:04:57]

> Sure. Unfortunately our work depends on them :(

:( indeed.  I suspect you'll get maximum happiness out of a whitelist of known
devices, or else do a sweep of KK devices as they get released.  (you've
previously said that JB-MR2 is needed for your stuff, but qinmin@ says upthread
that you actually need KK, not JB-MR2; if it's really JB-MR2 then that widens
the net of devices you should test for bugs in this area).

[commit-bot: I haz the power, 2013-11-12 23:54:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fischman@chromium.org/68923003/1

[Ami GONE FROM CHROMIUM, 2013-11-13 00:25:11]

Committed patchset #1 manually as r234675 (presubmit successful).