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

Submitted by Aman Gupta on Feb. 16, 2018, 3:52 a.m.

Details

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

Commit Message

Aman Gupta Feb. 16, 2018, 3:52 a.m.
From: Aman Gupta <aman@tmm1.net>

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(-)

Comments

Matthieu Bouron Feb. 16, 2018, 12:03 p.m.
On Thu, Feb 15, 2018 at 07:52:14PM -0800, Aman Gupta wrote:
> From: Aman Gupta <aman@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,
Aman Gupta Feb. 16, 2018, 5:40 p.m.
On Fri, Feb 16, 2018 at 4:01 AM Matthieu Bouron <matthieu.bouron@gmail.com>
wrote:

> On Thu, Feb 15, 2018 at 07:52:14PM -0800, Aman Gupta wrote:
> > From: Aman Gupta <aman@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)


Sure, that's probably better.


>
> > +        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.


Sounds good. I have been doing extensive device tests and everything is
working as I expect so far.

Aman


>
> Thanks,
>
> --
> Matthieu B.
>
Matthieu Bouron Feb. 19, 2018, 2:37 p.m.
On Fri, Feb 16, 2018 at 05:40:03PM +0000, Aman Gupta wrote:
> On Fri, Feb 16, 2018 at 4:01 AM Matthieu Bouron <matthieu.bouron@gmail.com>
> wrote:
> 
> > On Thu, Feb 15, 2018 at 07:52:14PM -0800, Aman Gupta wrote:
> > > From: Aman Gupta <aman@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)
> 
> 
> Sure, that's probably better.
> 
> 
> >
> > > +        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.
> 
> 
> Sounds good. I have been doing extensive device tests and everything is
> working as I expect so far.

Patch pushed.

Thanks,

Patch hide | download patch | download mbox

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;
+        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);