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
