Ho Ming Shun:
> MMAL is an fundamentally an asynchronous decoder, which was a bad fit
> for the legacy dataflow API. Often multiple packets are enqueued before
> a flood of frames are returned from MMAL.
>
> The previous lockstep dataflow meant that any delay in returning packets
> from the VPU would cause ctx->queue_decoded_frames to grow with no way
> of draining the queue.
>
> Testing this with mpv streaming from an RTSP source reduced decode
> latency from 2s to about 0.2s.
>
> Signed-off-by: Ho Ming Shun <cyph1...@gmail.com>
> ---
> libavcodec/mmaldec.c | 30 +++++++++++++++++++++++-------
> 1 file changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c
> index 96140bf53d..3d7cc90cd2 100644
> --- a/libavcodec/mmaldec.c
> +++ b/libavcodec/mmaldec.c
> @@ -570,6 +570,7 @@ static int ffmmal_add_packet(AVCodecContext *avctx,
> AVPacket *avpkt,
>
> done:
> av_buffer_unref(&buf);
> + av_packet_unref(avpkt);
> return ret;
> }
>
> @@ -655,6 +656,12 @@ static int ffmal_copy_frame(AVCodecContext *avctx,
> AVFrame *frame,
> avctx->pix_fmt, avctx->width, avctx->height);
> }
>
> + frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
> + frame->width = avctx->width;
> + frame->width = avctx->width;
> + frame->height = avctx->height;
> + frame->format = avctx->pix_fmt;
> +
> frame->pts = buffer->pts == MMAL_TIME_UNKNOWN ? AV_NOPTS_VALUE :
> buffer->pts;
> frame->pkt_dts = AV_NOPTS_VALUE;
>
> @@ -763,12 +770,12 @@ done:
> return ret;
> }
>
> -static int ffmmal_decode(AVCodecContext *avctx, void *data, int *got_frame,
> - AVPacket *avpkt)
> +static int ffmmal_receive_frame(AVCodecContext *avctx, AVFrame *frame)
> {
> MMALDecodeContext *ctx = avctx->priv_data;
> - AVFrame *frame = data;
> int ret = 0;
> + AVPacket avpkt;
You are adding a new AVPacket; and you are not even zeroing it. This is
even worse than the current code (and it might even be dangerous:
ff_decode_get_packet() expects initialized, blank packets, not
uninitialized ones; what you are doing only works because this decoder
does not have an automatically inserted bitstream filter).
You will have to add an actually allocated packet for this; or one could
reuse the spare packet of the DecodeSimpleContext that is unused for
decoders implementing the receive_frame API.
It is easy to fix the deprecation issue if one already has a spare
packet: Just put the extradata into said packet.
My guess that your patch does not exhibit any deep conflicts with mine
turned out to be correct: the only part where there is a real conflict
is in the fact that it doesn't make any sense any more to treat the
packet as const, given that a decoder implementing the receive_frame API
is supposed to unref the packets it receives on its own.
While I regard not wrapping the extradata in a packet as cleaner, the
code actually becomes simpler if one does so (as I will demonstrate
lateron). In other words: I drop my patches.
> + int got_frame = 0;
>
> if (avctx->extradata_size && !ctx->extradata_sent) {
> AVPacket pkt = {0};
> @@ -782,7 +789,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
> return ret;
> }
>
> - if ((ret = ffmmal_add_packet(avctx, avpkt, 0)) < 0)
> + ret = ff_decode_get_packet(avctx, &avpkt);
> + if(ret == 0) {
> + if ((ret = ffmmal_add_packet(avctx, &avpkt, 0)) < 0)
> + return ret;
> + } else if(ret < 0 && !(ret == AVERROR(EAGAIN)))
> return ret;
>
> if ((ret = ffmmal_fill_input_port(avctx)) < 0)
> @@ -791,7 +802,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
> if ((ret = ffmmal_fill_output_port(avctx)) < 0)
> return ret;
>
> - if ((ret = ffmmal_read_frame(avctx, frame, got_frame)) < 0)
> + if ((ret = ffmmal_read_frame(avctx, frame, &got_frame)) < 0)
> return ret;
>
> // ffmmal_read_frame() can block for a while. Since the decoder is
> @@ -803,7 +814,12 @@ FF_ENABLE_DEPRECATION_WARNINGS
> if ((ret = ffmmal_fill_input_port(avctx)) < 0)
> return ret;
>
> - return ret;
> + if(!got_frame && ret == 0)
> + return AVERROR(EAGAIN);
> + else
> + return ret;
> +
> +
Unnecessary newlines.
> }
>
> static const AVCodecHWConfigInternal *const mmal_hw_configs[] = {
> @@ -835,7 +851,7 @@ static const AVOption options[]={
> .priv_data_size = sizeof(MMALDecodeContext), \
> .init = ffmmal_init_decoder, \
> .close = ffmmal_close_decoder, \
> - .decode = ffmmal_decode, \
> + .receive_frame = ffmmal_receive_frame, \
> .flush = ffmmal_flush, \
> .priv_class = &ffmmal_##NAME##_dec_class, \
> .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, \
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
