[FFmpeg-devel] [PATCH 1/2] avcodec/mediacodecdec: add delay_flush option

[Matthieu Bouron]

On Tue, Mar 06, 2018 at 01:15:55PM -0800, Aman Gupta wrote:
> From: Aman Gupta <aman at tmm1.net>
> 
> The default behavior of the mediacodec decoder before this commit
> was to delay flushes until all pending hardware frames were
> returned to the decoder. This was useful for certain types of
> applications, but was unexpected behavior for others.
> 
> The new default behavior with this commit is now to execute
> flushes immediately to invalidate all pending frames. The old
> behavior can be enabled by setting delay_flush=1.
> 
> With the new behavior, video players implementing seek can simply
> call flush on the decoder without having to worry about whether
> they have one or more mediacodec frames still buffered in their
> rendering pipeline. Previously, all these frames had to be
> explictly freed (or rendered) before the seek/flush would execute.
> 
> The new behavior matches the behavior of all other lavc decoders,
> reducing the amount of special casing required when using the
> mediacodec decoder.
> ---
>  libavcodec/mediacodec.c           |  2 +-
>  libavcodec/mediacodecdec.c        | 23 +++++++++++++++++++++++
>  libavcodec/mediacodecdec_common.c | 11 ++++++++---
>  libavcodec/mediacodecdec_common.h |  4 ++++
>  4 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/mediacodec.c b/libavcodec/mediacodec.c
> index d492eefe0b..bf1b7477f1 100644
> --- a/libavcodec/mediacodec.c
> +++ b/libavcodec/mediacodec.c
> @@ -91,7 +91,7 @@ int av_mediacodec_release_buffer(AVMediaCodecBuffer *buffer, int render)
>      MediaCodecDecContext *ctx = buffer->ctx;
>      int released = atomic_fetch_add(&buffer->released, 1);
>  
> -    if (!released) {
> +    if (!released && (ctx->delay_flush || buffer->serial == atomic_load(&ctx->serial))) {
>          return ff_AMediaCodec_releaseOutputBuffer(ctx->codec, buffer->index, render);
>      }
>  
> diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
> index 0fe14846c3..3582a1377d 100644
> --- a/libavcodec/mediacodecdec.c
> +++ b/libavcodec/mediacodecdec.c
> @@ -41,10 +41,14 @@
>  
>  typedef struct MediaCodecH264DecContext {
>  
> +    AVClass *avclass;
> +
>      MediaCodecDecContext *ctx;
>  
>      AVPacket buffered_pkt;
>  
> +    int delay_flush;
> +
>  } MediaCodecH264DecContext;
>  
>  static av_cold int mediacodec_decode_close(AVCodecContext *avctx)
> @@ -366,6 +370,8 @@ static av_cold int mediacodec_decode_init(AVCodecContext *avctx)
>          goto done;
>      }
>  
> +    s->ctx->delay_flush = s->delay_flush;
> +
>      if ((ret = ff_mediacodec_dec_init(avctx, s->ctx, codec_mime, format)) < 0) {
>          s->ctx = NULL;
>          goto done;
> @@ -485,7 +491,24 @@ static const AVCodecHWConfigInternal *mediacodec_hw_configs[] = {
>      NULL
>  };
>  
> +#define OFFSET(x) offsetof(MediaCodecH264DecContext, x)
> +#define VD AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
> +static const AVOption ff_mediacodec_vdec_options[] = {
> +    { "delay_flush", "Delay flush until hw output buffers are returned to the decoder",
> +                     OFFSET(delay_flush), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VD },
> +    { NULL }
> +};
> +
> +#define DECLARE_MEDIACODEC_VCLASS(short_name)                   \
> +static const AVClass ff_##short_name##_mediacodec_dec_class = { \
> +    .class_name = #short_name "_mediacodec",                    \
> +    .item_name  = av_default_item_name,                         \
> +    .option     = ff_mediacodec_vdec_options,                   \
> +    .version    = LIBAVUTIL_VERSION_INT,                        \
> +};
> +
>  #define DECLARE_MEDIACODEC_VDEC(short_name, full_name, codec_id, bsf)                          \
> +DECLARE_MEDIACODEC_VCLASS(short_name)                                                          \
>  AVCodec ff_##short_name##_mediacodec_decoder = {                                               \
>      .name           = #short_name "_mediacodec",                                               \
>      .long_name      = NULL_IF_CONFIG_SMALL(full_name " Android MediaCodec decoder"),           \

The priv_class assignment is missing:

+    .priv_class     = &ff_##short_name##_mediacodec_dec_class, \

You will also need some kind of bump in the libavcodec library as well as
an APIChanges entry.

Other than that the patch looks good to me.

Thanks,

[...]

-- 
Matthieu B.