[FFmpeg-devel] [PATCH v9] lavf: palettized QuickTime video in Matroska

[Mats Peterson]

On 12/28/2015 12:25 PM, Mats Peterson wrote:
> On 12/28/2015 12:21 PM, Mats Peterson wrote:
>> On 12/28/2015 12:07 PM, Nicolas George wrote:
>>> L'octidi 8 nivôse, an CCXXIV, Mats Peterson a écrit :
>>>> Michael, he's talking about the OLD patch that was never applied. My
>>>> patch
>>>> has been written from scratch, more or less. I did borrowed some
>>>> palette
>>>> loops from mov.c, but I have also attributed the previous authors at
>>>> the top
>>>> of qtpalette.c properly.
>>>
>>> I must say, I find this hunk from 7973603:
>>>
>>> +        if (matroska->has_palette) {
>>> +            uint8_t *pal = av_packet_new_side_data(pkt,
>>> AV_PKT_DATA_PALETTE, AVPALETTE_SIZE);
>>> +            if (!pal) {
>>> +                av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append
>>> palette to packet\n");
>>> +            } else {
>>> +                memcpy(pal, matroska->palette, AVPALETTE_SIZE);
>>> +            }
>>> +            matroska->has_palette = 0;
>>> +        }
>>>
>>> looks quite similar to this hunk from
>>> https://trac.ffmpeg.org/attachment/ticket/5071/patchmkvmov.diff :
>>>
>>> +        if (matroska->pal) {
>>> +            uint8_t *pal = av_packet_new_side_data(pkt,
>>> AV_PKT_DATA_PALETTE, AVPALETTE_SIZE);
>>> +            if (!pal) {
>>> +                av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append
>>> palette to packet\n");
>>> +            } else {
>>> +                memcpy(pal, matroska->pal, AVPALETTE_SIZE);
>>> +            }
>>> +            av_freep(&matroska->pal);
>>> +        }
>>>
>>> Especially the use of if/else for error, and actually ignoring the
>>> error,
>>> instead of the most common (and more correct, but the rest of the code
>>> ignores error too) "if...return AVERROR(ENOMEM)".
>>>
>>> Regards,
>>>
>>>
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>
>> Yes it does, and that's why I said "more or less". That little chunk is
>> mimicked from mov.c for the record, so it's not his invention really.
>>
>
> The rest of what you said is hair-splitting in my book.
>

The important thing is not who wrote what (are you listening, Carl?) in 
the hunt for programming superstardom, but to provide a working solution.

Mats

-- 
Mats Peterson
http://matsp888.no-ip.org/~mats/