[FFmpeg-devel] [PATCH] qtpalette: make the color_* variables unsigned again

[Ganesh Ajjanagadde]

On Mon, Jan 11, 2016 at 9:35 PM, Mats Peterson
<matsp888-at-yahoo.com at ffmpeg.org> wrote:
> On 01/12/2016 03:32 AM, Mats Peterson wrote:
>>
>> On 01/12/2016 03:26 AM, Ronald S. Bultje wrote:
>>>
>>> Why are we using stdint types for non-vector data here? Our custom has
>>> always been to used sized (stdint-style) data only for vector data
>>> (arrays
>>> etc.), and use native-sized types (e.g. unsigned, int, whatever) for
>>> scalar
>>> values. Why are we making exceptions here?

That is not true; for instance while parsing headers, see avio_rl64.
In fact, really avio_rb32 and the like should return a uint32_t
instead of an unsigned int IMHO. There are of course a variety of
opinions on the subject of stdint types vs native-sized types; and I
doubt there is universal consensus on how liberally to use the stdint
sized types among FFmpeg developers.

>>>
>>> Ronald
>>
>>
>> Valid question. Of course there's no problem using uint32_t, but in the
>> original code the variables are unsigned int... ask Andreas ;)
>>
>> Mats
[...]
> You're free to make another patch, or if perhaps I should do it.

If something is inherently 32 bits (e.g obtained by reading 4 bytes),
then please don't make such a patch.
Seems to be the case here, and so I would nack such a patch:
color_start is obtained by an avio_rb32, keeping as uint32_t is
cleaner.

>
> Mats
>
> --
> Mats Peterson
> http://matsp888.no-ip.org/~mats/
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel