[FFmpeg-devel] [PATCH 2/2] lavc: add h264 mediacodec decoder

Matthieu Bouron matthieu.bouron at gmail.com
Thu Mar 3 14:03:01 CET 2016


On Wed, Mar 02, 2016 at 03:34:09PM +0100, Matthieu Bouron wrote:
> On Tue, Mar 01, 2016 at 11:26:45PM +0100, Michael Niedermayer wrote:
> > On Tue, Mar 01, 2016 at 08:01:45PM +0100, Matthieu Bouron wrote:
> > > On Sat, Feb 27, 2016 at 04:28:43PM +0100, Michael Niedermayer wrote:
> > > > On Fri, Feb 26, 2016 at 04:54:47PM +0100, Matthieu Bouron wrote:
> > > > > On Tue, Feb 23, 2016 at 11:28:01AM +0100, wm4 wrote:
> > > > > > On Tue, 23 Feb 2016 09:53:43 +0100
> > > > > > Matthieu Bouron <matthieu.bouron at gmail.com> wrote:
> > > > > > 
> > > > > > > On Mon, Feb 22, 2016 at 01:08:49PM +0100, Michael Niedermayer wrote:
> > > > > > > > On Mon, Feb 22, 2016 at 12:20:36PM +0100, Matthieu Bouron wrote:  
> > > > > > > > > From: Matthieu Bouron <matthieu.bouron at stupeflix.com>  
> > > > > > > > [...]  
> > > > > > > > > +        codec = (*env)->NewObject(env, jfields.mediacodec_list_class, jfields.init_id, 0);
> > > > > > > > > +        if (!codec) {
> > > > > > > > > +            av_log(NULL, AV_LOG_ERROR, "Could not create media codec list\n");
> > > > > > > > > +            goto done;
> > > > > > > > > +        }
> > > > > > > > > +
> > > > > > > > > +        tmp = (*env)->CallObjectMethod(env, codec, jfields.find_decoder_for_format_id, format);
> > > > > > > > > +        if (!tmp) {
> > > > > > > > > +            av_log(NULL, AV_LOG_ERROR, "Could not find decoder in media codec list\n");
> > > > > > > > > +            goto done;
> > > > > > > > > +        }
> > > > > > > > > +
> > > > > > > > > +        name = ff_jni_jstring_to_utf_chars(env, tmp, NULL);
> > > > > > > > > +        if (!name) {  
> > > > > > > >   
> > > > > > > > > +            av_log(NULL, AV_LOG_ERROR, "Could not convert jstring to utf chars\n");  
> > > > > > > > 
> > > > > > > > some non NULL context would be better, if possible, so the user knows
> > > > > > > > where an error came from  
> > > > > > > 
> > > > > > > It would require to pass the log_ctx (avctx) as an argument of
> > > > > > > ff_AMediaCodecList_getCodecByName.
> > > > > > > 
> > > > > > > All the functions of this wrapper does not take a log_ctx as argument
> > > > > > > to be as close as possible to the original NDK MediaCodec API. The
> > > > > > > NDK MediaCodec API does not provide any wrapper around MediaCodecList.
> > > > > > > 
> > > > > > > I would like the whole wrapper API to be consistent but also as close as
> > > > > > > possible to original one.
> > > > > > > If you think it's mandatory I can add a log_ctx argument to every
> > > > > > > functions of the wrapper API (so it will be used directly by av_log and
> > > > > > > ff_jni_exception_check).
> > > > > > > 
> > > > > > > On another note, I fixed locally this part of the code by adding missing calls
> > > > > > > to ff_jni_exception_check.
> > > > > > > 
> > > > > > > [...]
> > > > > > 
> > > > > > Is it possible to store the log_ctx somewhere in the JNI?
> > > > > > 
> > > > > > We might at some point in the future forbid av_log(NULL, ...) calls, and
> > > > > > then it'd get complicated.
> > > > > 
> > > > > Patch updated with the following differences:
> > > > >   * All logging arguments in mediacodec_wrapper functions are now non
> > > > >   NULL
> > > > >   * The mediacodec buffer copy functions has been moved to a separate file
> > > > >   * The Mediacodec enum codec flags are properly retrieved from JNI
> > > > > 
> > > > > The codec extradata conversion to annex b simplification has not been
> > > > > addressed in this update. The bitstream filter api does not seem to
> > > > > provide a way to do the extradata conversion without feeding an actual
> > > > > packet to the api (av_bitstream_filter_filter) and i would like to keep
> > > > > the codec initialization in the init function.
> > > > > 
> > > > > The patchset has been pushed to a new branch:
> > > > > https://github.com/mbouron/FFmpeg/tree/feature/mediacodec-support-v3
> > > > > 
> > > > > Matthieu
> > > > 
> > > > >  configure                         |    5 
> > > > >  libavcodec/Makefile               |    3 
> > > > >  libavcodec/allcodecs.c            |    1 
> > > > >  libavcodec/mediacodec_sw_buffer.c |  339 +++++++
> > > > >  libavcodec/mediacodec_sw_buffer.h |   62 +
> > > > >  libavcodec/mediacodec_wrapper.c   | 1674 ++++++++++++++++++++++++++++++++++++++
> > > > >  libavcodec/mediacodec_wrapper.h   |  122 ++
> > > > >  libavcodec/mediacodecdec.c        |  563 ++++++++++++
> > > > >  libavcodec/mediacodecdec.h        |   82 +
> > > > >  libavcodec/mediacodecdec_h264.c   |  356 ++++++++
> > > > >  10 files changed, 3207 insertions(+)
> > > > > f545068afece74d27cc04a365d9de7dcf5586a7d  0002-lavc-add-h264-mediacodec-decoder.patch
> > > > > From 805c7b95433f1bfbbc5d47fd59a1bbb755edb112 Mon Sep 17 00:00:00 2001
> > > > > From: Matthieu Bouron <matthieu.bouron at stupeflix.com>
> > > > > Date: Thu, 21 Jan 2016 09:29:39 +0100
> > > > > Subject: [PATCH 2/2] lavc: add h264 mediacodec decoder
> > > > 
> > > > breaks fate
> > > > swr-resample_lin-fltp-48000-44100
> > > > TEST    swr-resample_lin-dblp-8000-44100
> > > > TEST    swr-resample_lin-dblp-8000-48000
> > > > --- ./tests/ref/fate/source     2016-02-24 22:42:26.379879152 +0100
> > > > +++ tests/data/fate/source      2016-02-27 14:44:09.176735216 +0100
> > > > @@ -27,3 +27,4 @@
> > > >  compat/avisynth/windowsPorts/windows2linux.h
> > > >  compat/float/float.h
> > > >  compat/float/limits.h
> > > > +libavcodec/mediacodec_sw_buffer.h
> > > > Test source failed. Look at tests/data/fate/source.err for details.
> > > > make: *** [fate-source] Error 1
> > > > make: *** Waiting for unfinished jobs....
> > > 
> > > New patch attached fixing the issue. It also fixes an issue while the
> > > binding was trying to access the field MediaCodec.BUFFER_FLAG_KEY_FRAME
> > > which is only available in android >= 5 causing the JNI code to try to
> > > call a method on a NULL field ID which caused the VM to crash on older
> > > devices. This field is now non mandatory.
> > > 
> > > The codec extradata conversion is still not addressed as the bitstream API
> > > does not expose a method to do such conversion without feeding an actual
> > > packet to it.
> > > 
> > > I think what is left to do at this point is more testing towards corrupted
> > > bitstream and see how to the API behaves and see how it impacts the
> > > decoder (as I rely on counters of queued and dequeued buffers, which is a
> > > bit evil).
> > > If you happen to have such h264 samples, it would be very helpful.
> > 
> > its probably most efficient if you run the code under a fuzzer
> > 
> > fuzzed files which triggered other issues are likely not optimal nor
> > nummerous enough for hitting many different issues
> > 
> > the fate samples likely are a good source for undamaged h264 to fuzz
> > 
> 
> Patch updated with the following differences:
>   * The avctx width/height are now retrieved from MediaCodec using the
>   crop parameters the API returns, making the decoder handle properly
>   reinit-h264_420_8-to-small_420_8.h264
> 
> I'm currently testing the decoder against fuzzed samples. So far, the
> impact of a corrupted bitstream is having the decoder waiting 1second and
> then timing out while receiving a flush packet. It also outputs an error
> message like the following one:
> 
> Failed to dequeue output buffer within 1000ms, output will probably lack
> %d frames.
> 
> Is that acceptable to block for such a long time in this particular case ?
> 

Patch updated with the following differences:
  * ff_set_dimensions return code is now used
  * add missing exception when trying to call the MediaCodec object constructor
  * remove leftover avctx_internal field from MediaCodecH264DecContext
  * add ff_AMediaCodec_getName function

The dev branch can be found here:
https://github.com/mbouron/FFmpeg/tree/feature/mediacodec-support-v7

If nobody objects I would like to push the patchset in 3 days.

Matthieu

[...]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-lavc-add-h264-mediacodec-decoder.patch
Type: text/x-diff
Size: 113989 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160303/6eaf336a/attachment.patch>


More information about the ffmpeg-devel mailing list