[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