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.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.
Patch pushed.
Thanks,
--
Matthieu B.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
