1006285 - Problems with setting bitrate for HW H.264 encoder

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

Two problems have been noticed:  
1. If the bitrate was initially set to 2Mbps (via configure()), subsequent calls to setBitRate() don't seem to work; the rate remained too high.  I haven't gone back to re-verify since I recoded it to use configureDirect(); perhaps the way-over-bitrate at the start put it in a "bad" state that was hard to get out of.

2. If we use the new configureDirect() (or probably if tweak configure() to start at 300Kbps/10fps), then it appears to work - however, my measurements show that with a setting of 600, the bitrate appears to go over, generally wandering between 90000 and 120000 bits per frame at a configure input rate of 10fps.  When the average goes over 60000-75000 bits/frame or so, decoder delay jumps up from ~1s to ~1.5-1.75s (and the rate may drop below 10fps)

Note some addition verification of the input rate needs to be done, but the delta measurements on decoder events indicates 10fps.

Comment 1

[sachins]

To set the bitrate, MediaCodec::setParameters(AMessage params) should have a parameter key "video-bitrate". It seems like there is a typo in OMXCodecWrapper where the parameter name provided is "videoBitrate" [1].

[1] https://mxr.mozilla.org/mozilla-central/source/content/media/omx/OMXCodecWrapper.cpp#427

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

(In reply to sachins from comment #1)
> To set the bitrate, MediaCodec::setParameters(AMessage params) should have a
> parameter key "video-bitrate". It seems like there is a typo in
> OMXCodecWrapper where the parameter name provided is "videoBitrate" [1].

I'm having trouble finding clear documentation on what the allowed values are, but what I can find says "bitrate", not "video-bitrate" (and not "videoBitrate").  Do you have a link to docs for setParameters and the allowed settings names?

Comment 3

[John Lin [:jhlin][:jolin]]

Looks like the key has been changed from "videoBitrate" to "video-bitrate" at commit 530fdbdc [1]
The revision sync-ed by "config.sh flame" still uses "videoBitrate" so I suppose it should at least work on Flame.
However, this also means that in order to support different stagefright versions, we might need to maintain a list of correct key strings (probably in Gonk) and expose them to Gecko.

[1] https://android.googlesource.com/platform/frameworks/av/+/530fdbdc1b5491f3fbf172752834d1515701e142%5E!/#F0

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

> The revision sync-ed by "config.sh flame" still uses "videoBitrate" so I
> suppose it should at least work on Flame.
> However, this also means that in order to support different stagefright
> versions, we might need to maintain a list of correct key strings (probably
> in Gonk) and expose them to Gecko.

Ok, that explains why it appeared to work (after I changed the config code).  Is the new version in the KK builds being tried by Vikram/Diego?

We also could set *both* values.  Also, is there anything that keys us to the underlying version base of Gonk (KK or JB/etc)?  I presume there is.

Comment 5

[Michael Vines [:m1] [:evilmachines]]

> Is the new version in the KK builds being tried by Vikram/Diego?

Yes.

> Also, is there anything that keys us to the underlying version base of Gonk (KK or JB/etc)?

http://dxr.mozilla.org/mozilla-central/search?q=ANDROID_VERSION

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Attachment: make MediaCodec bitrate setting work for KitKat-based builds

Comment 7

[John Lin [:jhlin][:jolin]]

Comment on attachment 8420653 [details] [diff] [review]
make MediaCodec bitrate setting work for KitKat-based builds

Review of attachment 8420653 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/omx/OMXCodecWrapper.cpp
@@ +444,5 @@
>  {
>    sp<AMessage> msg = new AMessage();
> +#if ANDROID_VERSION >= 19
> +  // XXX Do we need a runtime check here?
> +  msg->setInt32("bitrate", aKbps * 1000 /* kbps -> bps */);

nit: s/bitrate/video-bitrate/

Comment 8

[sachins]

(In reply to Randell Jesup [:jesup] from comment #2)
> I'm having trouble finding clear documentation on what the allowed values
> are, but what I can find says "bitrate", not "video-bitrate" (and not
> "videoBitrate").  Do you have a link to docs for setParameters and the
> allowed settings names?
Seems like these parameters are only documented at Java SDK level [1] for MediaCodec as I believe MediaCodec is not supported at NDK level. For native side, you can always look at the ACodec::setParameters() 

[1] http://developer.android.com/reference/android/media/MediaCodec.html#PARAMETER_KEY_VIDEO_BITRATE

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/effb5d2a8111

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/effb5d2a8111