[FFmpeg-devel] [PATCH] IFF: Add grayscale support to decoder

[Ronald S. Bultje]

Hi,

On Mon, May 10, 2010 at 12:07 PM, Sebastian Vater
<cdgs.basty at googlemail.com> wrote:
> Ronald S. Bultje a ?crit :
>> On Mon, May 10, 2010 at 12:00 PM, Sebastian Vater
>> <cdgs.basty at googlemail.com> wrote:
>>> Ronald S. Bultje a ?crit :
>>>> On Mon, May 10, 2010 at 11:32 AM, Sebastian Vater
>>>> <cdgs.basty at googlemail.com> wrote:
>>>>> + ? ?if (count > 0) { // PIX_FMT_RGB8
>>>>>
>>>> if (count) { //...
>>>>
>>> It should really check for just > 0 (is signed)...if we start to play
>>> later with extradata stuff and have to subtract, it might be get < 0.
>>
>> Isn't count < 0 a bug?
>
> In both cases, the for loop which converts CMAP to lavc isn't executed,
> so it will create in both cases a grayscale palette.

I get that, but it doesn't answer the question: is count < 0 a bug, or
is that expected behaviour which we could reasonably expect to get in
the decoder?

If it is expected, then what does it mean?

If it is unexpected, then it's a bug and you should assert(count>=0);
and use the shorter code "if (count) { ..." because it's smaller
codesize.

> As said, it might make a difference when we add custom stuff to
> extradata (HAM/masking/etc.) when we are subtracting sth. from count it
> can get 0 (just see my HAM patch, where this breaks things if the just
> check count != 0).

But can it get < 0? And should it get < 0? And if so, what does that mean?

Ronald