On Thu, Feb 15, 2018 at 07:52:14PM -0800, Aman Gupta wrote: > From: Aman Gupta <a...@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. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel