On Fri, Feb 16, 2018 at 4:01 AM Matthieu Bouron <matthieu.bou...@gmail.com>
wrote:
> 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)
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.
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
