diff mbox

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

Message ID 20180306211556.3083-1-ffmpeg@tmm1.net
State New
Headers show

Commit Message

Aman Karmani March 6, 2018, 9:15 p.m. UTC
From: Aman Gupta <aman@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(-)

Comments

Matthieu Bouron March 7, 2018, 3:30 p.m. UTC | #1
On Tue, Mar 06, 2018 at 01:15:55PM -0800, Aman Gupta wrote:
> From: Aman Gupta <aman@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,

[...]
diff mbox

Patch

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"),           \
diff --git a/libavcodec/mediacodecdec_common.c b/libavcodec/mediacodecdec_common.c
index 929db78361..b4f1f6685b 100644
--- a/libavcodec/mediacodecdec_common.c
+++ b/libavcodec/mediacodecdec_common.c
@@ -178,11 +178,12 @@  static void mediacodec_buffer_release(void *opaque, uint8_t *data)
     MediaCodecDecContext *ctx = buffer->ctx;
     int released = atomic_load(&buffer->released);
 
-    if (!released) {
+    if (!released && (ctx->delay_flush || buffer->serial == atomic_load(&ctx->serial))) {
         ff_AMediaCodec_releaseOutputBuffer(ctx->codec, buffer->index, 0);
     }
 
-    ff_mediacodec_dec_unref(ctx);
+    if (ctx->delay_flush)
+        ff_mediacodec_dec_unref(ctx);
     av_freep(&buffer);
 }
 
@@ -236,7 +237,9 @@  FF_ENABLE_DEPRECATION_WARNINGS
     }
 
     buffer->ctx = s;
-    ff_mediacodec_dec_ref(s);
+    buffer->serial = atomic_load(&s->serial);
+    if (s->delay_flush)
+        ff_mediacodec_dec_ref(s);
 
     buffer->index = index;
     buffer->pts = info->presentationTimeUs;
@@ -425,6 +428,7 @@  static int mediacodec_dec_flush_codec(AVCodecContext *avctx, MediaCodecDecContex
     s->draining = 0;
     s->flushing = 0;
     s->eos = 0;
+    atomic_fetch_add(&s->serial, 1);
 
     status = ff_AMediaCodec_flush(codec);
     if (status < 0) {
@@ -449,6 +453,7 @@  int ff_mediacodec_dec_init(AVCodecContext *avctx, MediaCodecDecContext *s,
     };
 
     atomic_init(&s->refcount, 1);
+    atomic_init(&s->serial, 1);
 
     pix_fmt = ff_get_format(avctx, pix_fmts);
     if (pix_fmt == AV_PIX_FMT_MEDIACODEC) {
diff --git a/libavcodec/mediacodecdec_common.h b/libavcodec/mediacodecdec_common.h
index 85df507ffb..afb98ffed9 100644
--- a/libavcodec/mediacodecdec_common.h
+++ b/libavcodec/mediacodecdec_common.h
@@ -62,6 +62,9 @@  typedef struct MediaCodecDecContext {
 
     uint64_t output_buffer_count;
 
+    bool delay_flush;
+    atomic_int serial;
+
 } MediaCodecDecContext;
 
 int ff_mediacodec_dec_init(AVCodecContext *avctx,
@@ -93,6 +96,7 @@  typedef struct MediaCodecBuffer {
     ssize_t index;
     int64_t pts;
     atomic_int released;
+    int serial;
 
 } MediaCodecBuffer;