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

[Mats Peterson]

On 12/28/2015 03:29 PM, Michael Niedermayer wrote:
> On Mon, Dec 28, 2015 at 12:07:38PM +0100, 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)".
>
> for reference: (similar code prior to the patches)
>
> HEAD~20:libavdevice/x11grab.c-    pkt->pts  = curtime;
> HEAD~20:libavdevice/x11grab.c-    if (s->palette_changed) {
> HEAD~20:libavdevice/x11grab.c-        uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE,
> HEAD~20:libavdevice/x11grab.c-                                               AVPALETTE_SIZE);
> HEAD~20:libavdevice/x11grab.c-        if (!pal) {
> HEAD~20:libavdevice/x11grab.c:            av_log(s, AV_LOG_ERROR, "Cannot append palette to packet\n");
> HEAD~20:libavdevice/x11grab.c-        } else {
> HEAD~20:libavdevice/x11grab.c-            memcpy(pal, s->palette, AVPALETTE_SIZE);
> HEAD~20:libavdevice/x11grab.c-            s->palette_changed = 0;
> HEAD~20:libavdevice/x11grab.c-        }
> HEAD~20:libavdevice/x11grab.c-    }
> --
> HEAD~20:libavformat/asfdec_f.c-            if (asf_st->pkt.data && asf_st->palette_changed) {
> HEAD~20:libavformat/asfdec_f.c-                uint8_t *pal;
> HEAD~20:libavformat/asfdec_f.c-                pal = av_packet_new_side_data(&asf_st->pkt, AV_PKT_DATA_PALETTE,
> HEAD~20:libavformat/asfdec_f.c-                                              AVPALETTE_SIZE);
> HEAD~20:libavformat/asfdec_f.c-                if (!pal) {
> HEAD~20:libavformat/asfdec_f.c:                    av_log(s, AV_LOG_ERROR, "Cannot append palette to packet\n");
> HEAD~20:libavformat/asfdec_f.c-                } else {
> HEAD~20:libavformat/asfdec_f.c-                    memcpy(pal, asf_st->palette, AVPALETTE_SIZE);
> HEAD~20:libavformat/asfdec_f.c-                    asf_st->palette_changed = 0;
> HEAD~20:libavformat/asfdec_f.c-                }
> HEAD~20:libavformat/asfdec_f.c-            }
> --
> HEAD~20:libavformat/mov.c-        if (sc->has_palette) {
> HEAD~20:libavformat/mov.c-            uint8_t *pal;
> HEAD~20:libavformat/mov.c-
> HEAD~20:libavformat/mov.c-            pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE);
> HEAD~20:libavformat/mov.c-            if (!pal) {
> HEAD~20:libavformat/mov.c:                av_log(mov->fc, AV_LOG_ERROR, "Cannot append palette to packet\n");
> HEAD~20:libavformat/mov.c-            } else {
> HEAD~20:libavformat/mov.c-                memcpy(pal, sc->palette, AVPALETTE_SIZE);
> HEAD~20:libavformat/mov.c-                sc->has_palette = 0;
> HEAD~20:libavformat/mov.c-            }
> HEAD~20:libavformat/mov.c-        }
>
> [...]
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Interesting ;)

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