[FFmpeg-devel] [PATCH v3 1/5] avcodec/mmaldec: use decoupled dataflow

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2021-12-09 14:08:01)
>> From: Ho Ming Shun <cyph1984 at gmail.com>
>>
>> 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 a live RTSP source visibly reduced
>> latency introduced by frames waiting in queue_decoded_frames from
>> roughly 2s to 0.
>>
>> Signed-off-by: Ho Ming Shun <cyph1984 at gmail.com>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>> ---
>>  libavcodec/mmaldec.c | 32 +++++++++++++++++++++++++-------
>>  1 file changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c
>> index 8c7d749742..d336f10350 100644
>> --- a/libavcodec/mmaldec.c
>> +++ b/libavcodec/mmaldec.c
>> @@ -83,6 +83,8 @@ typedef struct MMALDecodeContext {
>>      // libavcodec API can't return new frames, and we have a logical deadlock.
>>      // This is avoided by queuing such buffers here.
>>      FFBufferEntry *waiting_buffers, *waiting_buffers_tail;
>> +    /* Packet used to hold received packets temporarily; not owned by us. */
>> +    AVPacket *pkt;
> 
> Why is this needed? Seems to be you'd save three lines of code (and,
> more importantly, a confusing pointer to an object you don't own) by
> simply directly accessing avctx->internal->in_pkt in
> ffmal_receive_frame().
> 

In principle, I don't want to hardcode where the temp packet comes from.
It wouldn't make a difference here (as there would only be only place
where avctx->internal->in_pkt is accessed), but I nevertheless like it
more this way.

- Andreas