[FFmpeg-cvslog] r25066 - trunk/libavcodec/imgconvert.c

[Måns Rullgård]

Baptiste Coudurier <baptiste.coudurier at gmail.com> writes:

> On 09/10/2010 11:06 AM, Reimar D?ffinger wrote:
>> On Fri, Sep 10, 2010 at 06:32:35PM +0100, M?ns Rullg?rd wrote:
>>> Reimar D?ffinger<Reimar.Doeffinger at gmx.de>  writes:
>>>
>>>> On Fri, Sep 10, 2010 at 05:28:27PM +0100, M?ns Rullg?rd wrote:
>>>>> Reimar D?ffinger<Reimar.Doeffinger at gmx.de>  writes:
>>>>>
>>>>>> On Fri, Sep 10, 2010 at 04:36:19PM +0100, M?ns Rullg?rd wrote:
>>>>>>> Why the FUCK do all 8-bit pixel formats claim they have a palette?
>>>>>>
>>>>>> Because anyone actually using (e.g. for displaying) the data
>>>>>> then doesn't need to add a special case for every single of them.
>>>>>
>>>>> But there is no palette.  How does pretending there is one when it
>>>>> isn't even allocated (hence the valgrind error) simplify anything?
>>>>
>>>> It's a bug if there is none, I know for sure that at least almost
>>>> all 8-bit formats did indeed have a palette in data[1] some time
>>>> ago.
>>>
>>> And just what is in that palette?
>>>
>>> In the case at hand, there is obviously _something_ there, but there's
>>> not enough of it.  Since you seem to think this makes sense, I invite
>>> you to fix the bug.  I'm not touching it further.
>>
>> I don't think there's anything at all there, it's just that crappy
>> hack of rawvideo decoder messing up (ok, ok, maybe I am being a bit
>> too extreme here).
>> av_image_fill_pointers just sets the palette pointer to right after
>> the image data. Which makes little sense and should probably at most
>> be done for PAL8 - otherwise it is inconsistent with avpicture_get_size.
>> Anyway that means in this case it points right into nothing.
>> rawdec.c needs a special case for all paletted formats and needs to set
>> data[1] to data initialized by ff_set_systematic_pal (normally
>> get_buffer would handle that, but that is not used for raw video).
>> A proper solution should of course clean the code up instead of making
>> it even more of a mess, but something like below code should be what
>> is needed:
>
> Well I never agreed to setting a palette for all 8 bits formats in the
> first place.
> IMHO If code using the pixel formats wants a palette it can generate
> it, instead of generating it systematically.

I fully agree.  I am still waiting for someone to explain what this
palette is expected to contain, given that the format does not use
one.

If this is another dirty hack to satisfy mplayer's twisted desires, I
think mplayer should just go and fuck itself, then die.

-- 
M?ns Rullg?rd
mans at mansr.com