[FFmpeg-devel] [PATCH v2] avcodec/h264_parser: Try to avoid (un)referencing

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2020-05-29 18:31:57)
>> When a slice is encountered, the H.264 parser up until now always
>> unreferenced and reset the currently active PPS; immediately
>> afterwards, the currently active PPS is set again which includes
>> referencing it. Given that it is typical for the active parameter
>> sets to change only seldomly, most of the time the new active PPS will
>> be the old one. Therefore this commit checks for this and only
>> unreferences the PPS if it changed.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>> ---
>> New and much simplified version of [1]. This has been made possible by
>> 5e316096fa9ba4493d9dbb48847ad8e0b0e188c3.
>>
>> [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-August/248374.html
>>
>>  libavcodec/h264_parser.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 
> I've been considering a utility function along the lines of:
> 
> int av_buffer_update(AVBufferRef **dst, AVBufferRef *src)
> {
>     if ((*dst)->buffer == src->buffer)

Obviously missing a check for *dst == NULL.

>         return 0;

What if *dst and src point to different parts of the same underlying
AVBuffer? You'd need to overwrite **dst with *src in this case.

>     av_buffer_unref(dst);
>     *dst = av_buffer_ref(src);
>     return 1;

Missing check for av_buffer_ref failure.

> }
> 
> which would help avoid unnecessary unrefs+refs in such cases. Thoughts?
> 
I also pondered such a helper function, but then opted for the simple
approach instead. But if you want such a function and if it resides in
libavutil/buffer.c, then one could improve this even further by not
calling av_buffer_unref(dst) which throws an AVBufferRef away, but by
just decrementing the refcount of the underlying AVBuffer of *dst (and
freeing it if necessary), incrementing the refcount of the AVBuffer of
src and overwriting **dst with *src.

- Andreas