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

Mats Peterson matsp888 at yahoo.com
Mon Dec 28 12:25:10 CET 2015


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.

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


More information about the ffmpeg-devel mailing list