[FFmpeg-devel] [PATCH] avcodec/mediacodecdec: refactor to take advantage of new decoding api

Matthieu Bouron matthieu.bouron at gmail.com
Fri Feb 16 14:03:13 EET 2018


On Thu, Feb 15, 2018 at 07:52:14PM -0800, Aman Gupta wrote:
> From: Aman Gupta <aman at tmm1.net>

Hi,

> 
> This refactor splits up the main mediacodec decode loop into two
> send/receive helpers, which are then used to rewrite the receive_frame
> callback and take full advantage of the new decoding api. Since we
> can now request packets on demand with ff_decode_get_packet(), the
> fifo buffer is no longer necessary and has been removed.
> 
> This change was motivated by behavior observed on certain Android TV
> devices, featuring hardware mpeg2/h264 decoders which also deinterlace
> content (to produce multiple frames per field). Previously, this code
> caused buffering issues because queueInputBuffer() was always invoked
> before each dequeueOutputBuffer(), even though twice as many output
> buffers were being generated.
> 
> With this patch, the decoder will always attempt to drain new frames
> first before sending more data into the underlying codec.
> ---
>  libavcodec/mediacodecdec.c        | 107 ++++++++++++++------------------------
>  libavcodec/mediacodecdec_common.c |  50 ++++++++++++------
>  libavcodec/mediacodecdec_common.h |  14 +++--
>  3 files changed, 80 insertions(+), 91 deletions(-)
> 
> diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
> index cb1151a195..0c29a1113d 100644
> --- a/libavcodec/mediacodecdec.c
> +++ b/libavcodec/mediacodecdec.c
> @@ -25,7 +25,6 @@
>  
>  #include "libavutil/avassert.h"
>  #include "libavutil/common.h"
> -#include "libavutil/fifo.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/pixfmt.h"
> @@ -43,8 +42,6 @@ typedef struct MediaCodecH264DecContext {
>  
>      MediaCodecDecContext *ctx;
>  
> -    AVFifoBuffer *fifo;
> -
>      AVPacket buffered_pkt;
>  
>  } MediaCodecH264DecContext;
> @@ -56,8 +53,6 @@ static av_cold int mediacodec_decode_close(AVCodecContext *avctx)
>      ff_mediacodec_dec_close(avctx, s->ctx);
>      s->ctx = NULL;
>  
> -    av_fifo_free(s->fifo);
> -
>      av_packet_unref(&s->buffered_pkt);
>  
>      return 0;
> @@ -400,12 +395,6 @@ static av_cold int mediacodec_decode_init(AVCodecContext *avctx)
>  
>      av_log(avctx, AV_LOG_INFO, "MediaCodec started successfully, ret = %d\n", ret);
>  
> -    s->fifo = av_fifo_alloc(sizeof(AVPacket));
> -    if (!s->fifo) {
> -        ret = AVERROR(ENOMEM);
> -        goto done;
> -    }
> -
>  done:
>      if (format) {
>          ff_AMediaFormat_delete(format);
> @@ -418,13 +407,33 @@ done:
>      return ret;
>  }
>  
> +static int mediacodec_send_receive(AVCodecContext *avctx,
> +                                   MediaCodecH264DecContext *s,
> +                                   AVFrame *frame, bool wait)
> +{
> +    int ret;
> +
> +    /* send any pending data from buffered packet */
> +    while (s->buffered_pkt.size) {
> +        ret = ff_mediacodec_dec_send(avctx, s->ctx, &s->buffered_pkt);
> +        if (ret == AVERROR(EAGAIN))
> +            break;
> +        if (ret < 0)
> +            return ret;

Maybe else if (ret < 0)

> +        s->buffered_pkt.size -= ret;
> +        s->buffered_pkt.data += ret;
> +        if (s->buffered_pkt.size <= 0)
> +            av_packet_unref(&s->buffered_pkt);
> +    }
> +
> +    /* check for new frame */
> +    return ff_mediacodec_dec_receive(avctx, s->ctx, frame, wait);
> +}
> +
>  static int mediacodec_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>  {
>      MediaCodecH264DecContext *s = avctx->priv_data;
>      int ret;
> -    int got_frame = 0;
> -    int is_eof = 0;
> -    AVPacket pkt = { 0 };
>  
>      /*
>       * MediaCodec.flush() discards both input and output buffers, thus we
> @@ -452,74 +461,34 @@ static int mediacodec_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>          }
>      }
>  
> -    ret = ff_decode_get_packet(avctx, &pkt);
> -    if (ret == AVERROR_EOF)
> -        is_eof = 1;
> -    else if (ret == AVERROR(EAGAIN))
> -        ; /* no input packet, but fallthrough to check for pending frames */
> -    else if (ret < 0)
> +    /* flush buffered packet and check for new frame */
> +    ret = mediacodec_send_receive(avctx, s, frame, false);
> +    if (ret != AVERROR(EAGAIN))
>          return ret;
>  
> -    /* buffer the input packet */
> -    if (pkt.size) {
> -        if (av_fifo_space(s->fifo) < sizeof(pkt)) {
> -            ret = av_fifo_realloc2(s->fifo,
> -                                   av_fifo_size(s->fifo) + sizeof(pkt));
> -            if (ret < 0) {
> -                av_packet_unref(&pkt);
> -                return ret;
> -            }
> -        }
> -        av_fifo_generic_write(s->fifo, &pkt, sizeof(pkt), NULL);
> -    }
> -
> -    /* process buffered data */
> -    while (!got_frame) {
> -        /* prepare the input data */
> -        if (s->buffered_pkt.size <= 0) {
> -            av_packet_unref(&s->buffered_pkt);
> -
> -            /* no more data */
> -            if (av_fifo_size(s->fifo) < sizeof(AVPacket)) {
> -                AVPacket null_pkt = { 0 };
> -                if (is_eof) {
> -                    ret = ff_mediacodec_dec_decode(avctx, s->ctx, frame,
> -                                                   &got_frame, &null_pkt);
> -                    if (ret < 0)
> -                        return ret;
> -                    else if (got_frame)
> -                        return 0;
> -                    else
> -                        return AVERROR_EOF;
> -                }
> -                return AVERROR(EAGAIN);
> -            }
> -
> -            av_fifo_generic_read(s->fifo, &s->buffered_pkt, sizeof(s->buffered_pkt), NULL);
> -        }
> +    /* skip fetching new packet if we still have one buffered */
> +    if (s->buffered_pkt.size > 0)
> +        return AVERROR(EAGAIN);
>  
> -        ret = ff_mediacodec_dec_decode(avctx, s->ctx, frame, &got_frame, &s->buffered_pkt);
> +    /* fetch new packet or eof */
> +    ret = ff_decode_get_packet(avctx, &s->buffered_pkt);
> +    if (ret == AVERROR_EOF) {
> +        AVPacket null_pkt = { 0 };
> +        ret = ff_mediacodec_dec_send(avctx, s->ctx, &null_pkt);
>          if (ret < 0)
>              return ret;
> -
> -        s->buffered_pkt.size -= ret;
> -        s->buffered_pkt.data += ret;
>      }
> +    else if (ret < 0)
> +        return ret;
>  
> -    return 0;
> +    /* crank decoder with new packet */
> +    return mediacodec_send_receive(avctx, s, frame, true);
>  }
>  
>  static void mediacodec_decode_flush(AVCodecContext *avctx)
>  {
>      MediaCodecH264DecContext *s = avctx->priv_data;
>  
> -    while (av_fifo_size(s->fifo)) {
> -        AVPacket pkt;
> -        av_fifo_generic_read(s->fifo, &pkt, sizeof(pkt), NULL);
> -        av_packet_unref(&pkt);
> -    }
> -    av_fifo_reset(s->fifo);
> -
>      av_packet_unref(&s->buffered_pkt);
>  
>      ff_mediacodec_dec_flush(avctx, s->ctx);
> diff --git a/libavcodec/mediacodecdec_common.c b/libavcodec/mediacodecdec_common.c
> index a9147f3a08..b44abaef7f 100644
> --- a/libavcodec/mediacodecdec_common.c
> +++ b/libavcodec/mediacodecdec_common.c
> @@ -555,23 +555,17 @@ fail:
>      return ret;
>  }
>  
> -int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
> -                             AVFrame *frame, int *got_frame,
> -                             AVPacket *pkt)
> +int ff_mediacodec_dec_send(AVCodecContext *avctx, MediaCodecDecContext *s,
> +                           AVPacket *pkt)
>  {
> -    int ret;
>      int offset = 0;
>      int need_draining = 0;
>      uint8_t *data;
>      ssize_t index;
>      size_t size;
>      FFAMediaCodec *codec = s->codec;
> -    FFAMediaCodecBufferInfo info = { 0 };
> -
>      int status;
> -
>      int64_t input_dequeue_timeout_us = INPUT_DEQUEUE_TIMEOUT_US;
> -    int64_t output_dequeue_timeout_us = OUTPUT_DEQUEUE_TIMEOUT_US;
>  
>      if (s->flushing) {
>          av_log(avctx, AV_LOG_ERROR, "Decoder is flushing and cannot accept new buffer "
> @@ -584,13 +578,14 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
>      }
>  
>      if (s->draining && s->eos) {
> -        return 0;
> +        return AVERROR_EOF;
>      }
>  
>      while (offset < pkt->size || (need_draining && !s->draining)) {
>  
>          index = ff_AMediaCodec_dequeueInputBuffer(codec, input_dequeue_timeout_us);
>          if (ff_AMediaCodec_infoTryAgainLater(codec, index)) {
> +            av_log(avctx, AV_LOG_TRACE, "Failed to dequeue input buffer, try again later..\n");
>              break;
>          }
>  
> @@ -621,13 +616,15 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
>                  return AVERROR_EXTERNAL;
>              }
>  
> +            av_log(avctx, AV_LOG_TRACE, "Queued input buffer %zd"
> +                    " size=%zd ts=%" PRIi64 "\n", index, size, pts);
> +
>              s->draining = 1;
>              break;
>          } else {
>              int64_t pts = pkt->pts;
>  
>              size = FFMIN(pkt->size - offset, size);
> -
>              memcpy(data, pkt->data + offset, size);
>              offset += size;
>  
> @@ -643,11 +640,32 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
>          }
>      }
>  
> -    if (need_draining || s->draining) {
> +    if (offset == 0)
> +        return AVERROR(EAGAIN);
> +    return offset;
> +}
> +
> +int ff_mediacodec_dec_receive(AVCodecContext *avctx, MediaCodecDecContext *s,
> +                              AVFrame *frame, bool wait)
> +{
> +    int ret;
> +    uint8_t *data;
> +    ssize_t index;
> +    size_t size;
> +    FFAMediaCodec *codec = s->codec;
> +    FFAMediaCodecBufferInfo info = { 0 };
> +    int status;
> +    int64_t output_dequeue_timeout_us = OUTPUT_DEQUEUE_TIMEOUT_US;
> +
> +    if (s->draining && s->eos) {
> +        return AVERROR_EOF;
> +    }
> +
> +    if (s->draining) {
>          /* If the codec is flushing or need to be flushed, block for a fair
>           * amount of time to ensure we got a frame */
>          output_dequeue_timeout_us = OUTPUT_DEQUEUE_BLOCK_TIMEOUT_US;
> -    } else if (s->output_buffer_count == 0) {
> +    } else if (s->output_buffer_count == 0 || !wait) {
>          /* If the codec hasn't produced any frames, do not block so we
>           * can push data to it as fast as possible, and get the first
>           * frame */
> @@ -656,9 +674,7 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
>  
>      index = ff_AMediaCodec_dequeueOutputBuffer(codec, &info, output_dequeue_timeout_us);
>      if (index >= 0) {
> -        int ret;
> -
> -        av_log(avctx, AV_LOG_DEBUG, "Got output buffer %zd"
> +        av_log(avctx, AV_LOG_TRACE, "Got output buffer %zd"
>                  " offset=%" PRIi32 " size=%" PRIi32 " ts=%" PRIi64
>                  " flags=%" PRIu32 "\n", index, info.offset, info.size,
>                  info.presentationTimeUs, info.flags);
> @@ -686,8 +702,8 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
>                  }
>              }
>  
> -            *got_frame = 1;
>              s->output_buffer_count++;
> +            return 0;
>          } else {
>              status = ff_AMediaCodec_releaseOutputBuffer(codec, index, 0);
>              if (status < 0) {
> @@ -737,7 +753,7 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
>          return AVERROR_EXTERNAL;
>      }
>  
> -    return offset;
> +    return AVERROR(EAGAIN);
>  }
>  
>  int ff_mediacodec_dec_flush(AVCodecContext *avctx, MediaCodecDecContext *s)
> diff --git a/libavcodec/mediacodecdec_common.h b/libavcodec/mediacodecdec_common.h
> index 10f38277b5..32d16d3e3a 100644
> --- a/libavcodec/mediacodecdec_common.h
> +++ b/libavcodec/mediacodecdec_common.h
> @@ -25,6 +25,7 @@
>  
>  #include <stdint.h>
>  #include <stdatomic.h>
> +#include <stdbool.h>
>  #include <sys/types.h>
>  
>  #include "libavutil/frame.h"
> @@ -69,11 +70,14 @@ int ff_mediacodec_dec_init(AVCodecContext *avctx,
>                             const char *mime,
>                             FFAMediaFormat *format);
>  
> -int ff_mediacodec_dec_decode(AVCodecContext *avctx,
> -                             MediaCodecDecContext *s,
> -                             AVFrame *frame,
> -                             int *got_frame,
> -                             AVPacket *pkt);
> +int ff_mediacodec_dec_send(AVCodecContext *avctx,
> +                           MediaCodecDecContext *s,
> +                           AVPacket *pkt);
> +
> +int ff_mediacodec_dec_receive(AVCodecContext *avctx,
> +                              MediaCodecDecContext *s,
> +                              AVFrame *frame,
> +                              bool wait);
>  
>  int ff_mediacodec_dec_flush(AVCodecContext *avctx,
>                              MediaCodecDecContext *s);
> -- 
> 2.14.2
> 

The patch LGTM and passes my MediaCodec tests (tested on a OnePlus 5T). I
plan to test it on more devices on Monday before pushing it if that is OK
with you.

Thanks,

-- 
Matthieu B.


More information about the ffmpeg-devel mailing list