[FFmpeg-devel] [PATCH v3 3/3] lavc/libdavs2.c: use decoder data directly instead of memcpy

[Marton Balint]

On Sat, 13 Jul 2019, hwren wrote:

>
> At 2019-07-13 01:38:55, "Marton Balint" <cus at passwd.hu> wrote:
>>
>>
>> On Fri, 12 Jul 2019, hwrenx wrote:
>>
>>> Can effectivly improved decoding speed when memcpy becomes a limitation
>>> for proccessing high resolution source.
>>> Tested under i7-8700k with `ffmpeg -i 7680x4320.avs2 -vsync 0 -f null -`
>>> got performance 23fps => 42fps
>>>
>>> Signed-off-by: hwrenx <hwrenx at 126.com>
>>> ---
>>> libavcodec/libdavs2.c | 54 +++++++++++++++++++++++++++++----------------------
>>> 1 file changed, 31 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
>>> index 1b274a3..af0778f 100644
>>> --- a/libavcodec/libdavs2.c
>>> +++ b/libavcodec/libdavs2.c
>>> @@ -60,13 +60,24 @@ static av_cold int davs2_init(AVCodecContext *avctx)
>>>     return 0;
>>> }
>>> 
>>> +static void davs2_frame_unref(void *opaque, uint8_t *data) {
>>> +    DAVS2Context    *cad = (DAVS2Context *)opaque;
>>> +    davs2_picture_t  pic;
>>> +
>>> +    pic.magic = (davs2_picture_t *)data;
>>> +
>>> +    if (cad->decoder) {
>>> +        davs2_decoder_frame_unref(cad->decoder, &pic);
>>> +    } else {
>>> +        av_log(NULL, AV_LOG_WARNING, "Decoder not found, frame unreference failed.\n");
>>
>> Whoa, this should not happen, and you have to be prepared that the user 
>> might close the decoder before freeing the last frame.
>
> I hope so, actually it's harmless if we failed to unref some frames outside davs2 when decoder
> was closed through davs2_decoder_close(). The decoder will free the memory which contains
> all recycle data itself.

And that is the problem, because the data should survive closing the 
(avcodec) decoder.

>
>>
>> Maybe you should use some refcounting and create references to the 
>> decoder, others may have a better idea.
>>
>> Regards,
>> Marton
>
> Of course, anyway, it would be better if there are any ways to make sure we correctly freed all
> frames even decoder was not found or exited without a davs2_decoder_close().  But force to
> free the memory immediately in the call back function can cause troubles to decoder and I'm
> not very clearly understand how to use reference to deal with this problem inside a codec.
>
>
> Maybe that means I could reference the buffer and waiting ffmpeg to free them once they were
> not handled by davs2?

I am not sure I understand your concern here.

Here is what I propose in more detail, I think it works for all cases:

In davs_init allocate a small struct which contains the decoder and a 
reference counter for the decoder itself:
DAVS2Reference {
   atomic_int refcount;
   void *decoder;
}
set the refs to 1.

In davs2_dump_frames you should pass the allocated DAVS2Reference to 
av_buffer_create instead of the DAVS2Context. After each successful 
av_buffer_create you should increase refcount.

In davs2_frame_unref after freeing the frame you should decrease the 
reference counter and if you reach 0 then you can destroy the davs2 
decoder (and free DAVS2Reference struct as well)

You can do the same in davs2_end: decrease the refcount and if you reach 0 
then you can close the davs2 decoder and free the DAVS2Reference struct.

Regards,
Marton