[FFmpeg-devel] [PATCH 3/5] avcodec: Factor updating palette out

[Andreas Rheinhardt]

Andreas Rheinhardt:
> James Almer:
>> On 3/17/2021 8:59 PM, Andreas Rheinhardt wrote:
>>> Because the properties of frames returned from ff_get/reget_buffer
>>> are not reset at all, lots of returned frames had palette_has_changed
>>> wrongly set to 1. This has been changed, too.
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>> ---
>>>   libavcodec/8bps.c           | 11 +----------
>>>   libavcodec/cinepak.c        |  9 +--------
>>>   libavcodec/decode.c         | 14 ++++++++++++++
>>>   libavcodec/gdv.c            |  5 +----
>>>   libavcodec/idcinvideo.c     |  9 +--------
>>>   libavcodec/imx.c            |  5 +----
>>>   libavcodec/internal.h       |  9 +++++++++
>>>   libavcodec/interplayvideo.c |  9 +--------
>>>   libavcodec/kmvc.c           |  8 +-------
>>>   libavcodec/msrle.c          | 11 ++---------
>>>   libavcodec/msvideo1.c       | 10 +---------
>>>   libavcodec/qpeg.c           |  9 +--------
>>>   libavcodec/qtrle.c          | 10 +---------
>>>   libavcodec/rawdec.c         | 13 ++-----------
>>>   libavcodec/rscc.c           | 13 ++-----------
>>>   libavcodec/smc.c            |  9 +--------
>>>   libavcodec/tscc.c           | 10 +---------
>>>   17 files changed, 41 insertions(+), 123 deletions(-)
>>>
>>
>> [...]
>>
>>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>>> index 5a00aeedae..efa8a9ac8d 100644
>>> --- a/libavcodec/decode.c
>>> +++ b/libavcodec/decode.c
>>> @@ -2051,3 +2051,17 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>>         return 0;
>>>   }
>>> +
>>> +int ff_copy_palette(void *dst, const AVPacket *src, void *logctx)
>>
>> All the arguments for dst are uint8_t*.
> 
> Actually, most of them are uint32_t*. The only (?) exception is
> rawdec.c. (An earlier version of this patch used "uint32_t
> dst[AVPALETTE_SIZE]" with a cast for rawdec, but then I noticed that
> this is of by a factor of sizeof(uint32_t).)
> 

rscc.c is also an exception.

>>
>>> +{
>>> +    buffer_size_t size;
>>> +    const void *pal = av_packet_get_side_data(src,
>>> AV_PKT_DATA_PALETTE, &size);
>>
>> Same, av_packet_get_side_data() returns an uint8_t*.
> 
> Yes, but it actually is an array of uint32_t, hence void*. (void* is a
> better return value for av_packet_get_side_data() anyway, as most of the
> side data types are structures. Maybe we should change that.)
> 
>>
>>> +
>>> +    if (pal && size == AVPALETTE_SIZE) {
>>> +        memcpy(dst, pal, AVPALETTE_SIZE);
>>> +        return 1;
>>> +    } else if (pal) {
>>> +        av_log(logctx, AV_LOG_ERROR, "Palette size %d is wrong\n",
>>> size);
>>> +    }
>>> +    return 0;
>>> +}
>>
>> [...]
>>
>>> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
>>> index b57b996816..0fb3107979 100644
>>> --- a/libavcodec/internal.h
>>> +++ b/libavcodec/internal.h
>>> @@ -393,6 +393,15 @@ int ff_int_from_list_or_default(void *ctx, const
>>> char * val_name, int val,
>>>     void ff_dvdsub_parse_palette(uint32_t *palette, const char *p);
>>>   +/**
>>> + * Check whether the side-data of src contains a palette of
>>> + * size AVPALETTE_SIZE; if so, copy it to dst and return 1;
>>> + * else return 0.
>>> + * Also emit an error message upon encountering a palette
>>> + * with invalid size.
>>> + */
>>> +int ff_copy_palette(void *dst, const AVPacket *src, void *logctx);
>>
>> Should be in libavcodec/decode.h instead.
> 
> It was in decode.h until I noticed that most decoders don't include that
> header. internal.h includes several other functions that are only used
> by decoders (like ff_reget_buffer). But, yes, will move it.
> 
>>
>> Or maybe avpacket.c and packet_internal.h, for that matter.
> 
> Given that it is only used by decoders, decode.c seems the appropriate
> place (is it noticeable that I dream of a day when all the decoding code
> is really disabled when there are no decoders enabled?).
> 
>>
>>> +
>>>   #if defined(_WIN32) && CONFIG_SHARED && !defined(BUILDING_avcodec)
>>>   #    define av_export_avcodec __declspec(dllimport)
>>>   #else
>