[FFmpeg-devel] [PATCH v2] libavcodec/mediacodecdec: switch to new decoder api
Matthieu Bouron
matthieu.bouron at gmail.com
Wed Jan 3 13:25:30 EET 2018
On Thu, Dec 28, 2017 at 05:33:14PM -0800, Aman Gupta wrote:
> From: Aman Gupta <aman at tmm1.net>
>
> Using the new API gives the decoder the ability to produce
> N frames per input packet. This is particularly useful with
> mpeg2 decoders on some android devices, which automatically
> deinterlace video and produce one frame per field.
>
> Signed-off-by: Aman Gupta <aman at tmm1.net>
> ---
> libavcodec/mediacodecdec.c | 77 +++++++++++++++++++++++++++-------------------
> 1 file changed, 45 insertions(+), 32 deletions(-)
Hi,
Thanks for the patch.
I'm attaching a new version of your patch. The new version fixes some
issues that appear while seeking or when the decoder reaches EOF.
The following comments correspond to the changes I made in the new patch
(except for a few minor cosmetics like = {0}; -> = { 0 };).
>
> diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
> index b698ceaef9..90497c64da 100644
> --- a/libavcodec/mediacodecdec.c
> +++ b/libavcodec/mediacodecdec.c
> @@ -31,6 +31,7 @@
> #include "libavutil/pixfmt.h"
>
> #include "avcodec.h"
> +#include "decode.h"
> #include "h264_parse.h"
> #include "hevc_parse.h"
> #include "hwaccel.h"
> @@ -424,29 +425,13 @@ static int mediacodec_process_data(AVCodecContext *avctx, AVFrame *frame,
> return ff_mediacodec_dec_decode(avctx, s->ctx, frame, got_frame, pkt);
> }
>
> -static int mediacodec_decode_frame(AVCodecContext *avctx, void *data,
> - int *got_frame, AVPacket *avpkt)
> +static int mediacodec_receive_frame(AVCodecContext *avctx, AVFrame *frame)
> {
> MediaCodecH264DecContext *s = avctx->priv_data;
> - AVFrame *frame = data;
> int ret;
> -
> - /* buffer the input packet */
> - if (avpkt->size) {
> - AVPacket input_pkt = { 0 };
> -
> - if (av_fifo_space(s->fifo) < sizeof(input_pkt)) {
> - ret = av_fifo_realloc2(s->fifo,
> - av_fifo_size(s->fifo) + sizeof(input_pkt));
> - if (ret < 0)
> - return ret;
> - }
> -
> - ret = av_packet_ref(&input_pkt, avpkt);
> - if (ret < 0)
> - return ret;
> - av_fifo_generic_write(s->fifo, &input_pkt, sizeof(input_pkt), NULL);
> - }
> + int got_frame = 0;
> + int is_eof = 0;
> + AVPacket pkt = {0};
>
> /*
> * MediaCodec.flush() discards both input and output buffers, thus we
> @@ -470,26 +455,51 @@ static int mediacodec_decode_frame(AVCodecContext *avctx, void *data,
> */
> if (ff_mediacodec_dec_is_flushing(avctx, s->ctx)) {
> if (!ff_mediacodec_dec_flush(avctx, s->ctx)) {
> - return avpkt->size;
> + return 0;
> }
> + return AVERROR(EAGAIN);
You should only return AVERROR(EAGAIN) if ff_mediacodec_dec_flush returns
0:
if (ff_mediacodec_dec_is_flushing(avctx, s->ctx)) {
if (!ff_mediacodec_dec_flush(avctx, s->ctx)) {
return AVERROR(EAGAIN);
}
}
> + }
> +
> + 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)
> + 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) {
> + 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)) {
> - return avpkt->size ? avpkt->size :
> - ff_mediacodec_dec_decode(avctx, s->ctx, frame, got_frame, avpkt);
> + AVPacket null_pkt = {0};
> + if (is_eof)
> + return ff_mediacodec_dec_decode(avctx, s->ctx, frame,
> + &got_frame, &null_pkt);
Returning the return value of ff_mediacodec_dec_decode is not valid. It
should be something like:
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);
> }
>
> - ret = mediacodec_process_data(avctx, frame, got_frame, &s->buffered_pkt);
> + ret = mediacodec_process_data(avctx, frame, &got_frame, &s->buffered_pkt);
> if (ret < 0)
> return ret;
>
> @@ -497,7 +507,10 @@ static int mediacodec_decode_frame(AVCodecContext *avctx, void *data,
> s->buffered_pkt.data += ret;
> }
>
> - return avpkt->size;
> + if (got_frame)
> + return 0;
> +
> + return AVERROR(EAGAIN);
got_frame is always 1 here, returning 0 should be enough.
[...]
Let me know if the attached updated patch works for you.
Best regards,
--
Matthieu B.
-------------- next part --------------
>From 1f2bd9721973de9a4d83f5b2c9a5f1de793f641d Mon Sep 17 00:00:00 2001
From: Aman Gupta <aman at tmm1.net>
Date: Thu, 28 Dec 2017 17:33:14 -0800
Subject: [PATCH] libavcodec/mediacodecdec: switch to new decoder api
Using the new API gives the decoder the ability to produce
N frames per input packet. This is particularly useful with
mpeg2 decoders on some android devices, which automatically
deinterlace video and produce one frame per field.
Signed-off-by: Aman Gupta <aman at tmm1.net>
Signed-off-by: Matthieu Bouron <matthieu.bouron at gmail.com>
---
libavcodec/mediacodecdec.c | 80 +++++++++++++++++++++++++++-------------------
1 file changed, 48 insertions(+), 32 deletions(-)
diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
index 3460478f43..94bff039a6 100644
--- a/libavcodec/mediacodecdec.c
+++ b/libavcodec/mediacodecdec.c
@@ -31,6 +31,7 @@
#include "libavutil/pixfmt.h"
#include "avcodec.h"
+#include "decode.h"
#include "h264_parse.h"
#include "hevc_parse.h"
#include "hwaccel.h"
@@ -442,29 +443,13 @@ static int mediacodec_process_data(AVCodecContext *avctx, AVFrame *frame,
return ff_mediacodec_dec_decode(avctx, s->ctx, frame, got_frame, pkt);
}
-static int mediacodec_decode_frame(AVCodecContext *avctx, void *data,
- int *got_frame, AVPacket *avpkt)
+static int mediacodec_receive_frame(AVCodecContext *avctx, AVFrame *frame)
{
MediaCodecH264DecContext *s = avctx->priv_data;
- AVFrame *frame = data;
int ret;
-
- /* buffer the input packet */
- if (avpkt->size) {
- AVPacket input_pkt = { 0 };
-
- if (av_fifo_space(s->fifo) < sizeof(input_pkt)) {
- ret = av_fifo_realloc2(s->fifo,
- av_fifo_size(s->fifo) + sizeof(input_pkt));
- if (ret < 0)
- return ret;
- }
-
- ret = av_packet_ref(&input_pkt, avpkt);
- if (ret < 0)
- return ret;
- av_fifo_generic_write(s->fifo, &input_pkt, sizeof(input_pkt), NULL);
- }
+ int got_frame = 0;
+ int is_eof = 0;
+ AVPacket pkt = { 0 };
/*
* MediaCodec.flush() discards both input and output buffers, thus we
@@ -488,26 +473,57 @@ static int mediacodec_decode_frame(AVCodecContext *avctx, void *data,
*/
if (ff_mediacodec_dec_is_flushing(avctx, s->ctx)) {
if (!ff_mediacodec_dec_flush(avctx, s->ctx)) {
- return avpkt->size;
+ return AVERROR(EAGAIN);
}
}
+ 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)
+ 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) {
+ 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)) {
- return avpkt->size ? avpkt->size :
- ff_mediacodec_dec_decode(avctx, s->ctx, frame, got_frame, avpkt);
+ 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);
}
- ret = mediacodec_process_data(avctx, frame, got_frame, &s->buffered_pkt);
+ ret = mediacodec_process_data(avctx, frame, &got_frame, &s->buffered_pkt);
if (ret < 0)
return ret;
@@ -515,7 +531,7 @@ static int mediacodec_decode_frame(AVCodecContext *avctx, void *data,
s->buffered_pkt.data += ret;
}
- return avpkt->size;
+ return 0;
}
static void mediacodec_decode_flush(AVCodecContext *avctx)
@@ -555,7 +571,7 @@ AVCodec ff_h264_mediacodec_decoder = {
.id = AV_CODEC_ID_H264,
.priv_data_size = sizeof(MediaCodecH264DecContext),
.init = mediacodec_decode_init,
- .decode = mediacodec_decode_frame,
+ .receive_frame = mediacodec_receive_frame,
.flush = mediacodec_decode_flush,
.close = mediacodec_decode_close,
.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HARDWARE,
@@ -574,7 +590,7 @@ AVCodec ff_hevc_mediacodec_decoder = {
.id = AV_CODEC_ID_HEVC,
.priv_data_size = sizeof(MediaCodecH264DecContext),
.init = mediacodec_decode_init,
- .decode = mediacodec_decode_frame,
+ .receive_frame = mediacodec_receive_frame,
.flush = mediacodec_decode_flush,
.close = mediacodec_decode_close,
.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HARDWARE,
@@ -593,7 +609,7 @@ AVCodec ff_mpeg2_mediacodec_decoder = {
.id = AV_CODEC_ID_MPEG2VIDEO,
.priv_data_size = sizeof(MediaCodecH264DecContext),
.init = mediacodec_decode_init,
- .decode = mediacodec_decode_frame,
+ .receive_frame = mediacodec_receive_frame,
.flush = mediacodec_decode_flush,
.close = mediacodec_decode_close,
.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HARDWARE,
@@ -611,7 +627,7 @@ AVCodec ff_mpeg4_mediacodec_decoder = {
.id = AV_CODEC_ID_MPEG4,
.priv_data_size = sizeof(MediaCodecH264DecContext),
.init = mediacodec_decode_init,
- .decode = mediacodec_decode_frame,
+ .receive_frame = mediacodec_receive_frame,
.flush = mediacodec_decode_flush,
.close = mediacodec_decode_close,
.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HARDWARE,
@@ -629,7 +645,7 @@ AVCodec ff_vp8_mediacodec_decoder = {
.id = AV_CODEC_ID_VP8,
.priv_data_size = sizeof(MediaCodecH264DecContext),
.init = mediacodec_decode_init,
- .decode = mediacodec_decode_frame,
+ .receive_frame = mediacodec_receive_frame,
.flush = mediacodec_decode_flush,
.close = mediacodec_decode_close,
.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HARDWARE,
@@ -647,7 +663,7 @@ AVCodec ff_vp9_mediacodec_decoder = {
.id = AV_CODEC_ID_VP9,
.priv_data_size = sizeof(MediaCodecH264DecContext),
.init = mediacodec_decode_init,
- .decode = mediacodec_decode_frame,
+ .receive_frame = mediacodec_receive_frame,
.flush = mediacodec_decode_flush,
.close = mediacodec_decode_close,
.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AVOID_PROBING | AV_CODEC_CAP_HARDWARE,
--
2.15.1
More information about the ffmpeg-devel
mailing list