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

[Mats Peterson]

On 12/27/2015 04:42 AM, Mats Peterson wrote:
> On 12/27/2015 03:57 AM, Mats Peterson wrote:
>> On 12/27/2015 03:03 AM, Michael Niedermayer wrote:
>>>> +
>>>> +    if (!(stsd = av_malloc(70)))
>>>> +        return AVERROR(ENOMEM);
>>>
>>> the malloc is unneeded, an array on the stack could be used (its just
>>> a fixed 70 bytes)
>>> this would also simplify the error handling
>>
>> Yes, I thought so. I tried to be "a good boy", but that was obviously to
>> no avail ;)
>>
>>>> +int ff_get_qtpalette(int codec_id, uint8_t *stsd, AVIOContext *pb,
>>>> +        uint32_t *palette);
>>>> +
>>>
>>> missing doxy documentation, missing "const" for unchanged arrays
>>> also why does this need a "byte" array and a AVIOContext as input
>>> arguments ?
>>> iam asking as this looks a bit confusing with 2 inputs ...
>>
>> Regarding doxy documentation, I notice several files in libavformat are
>> lacking doxy documentation (if what you mean by "doxy documentation" is
>> a comment beginning with /**). I don't know what to put it in either, at
>> that. Please help me out.
>>
>> And regarding two inputs, well, the problem is that matroskadec.c has
>> the video sample description stored in its in-memory private data, while
>> mov.c reads the video sample description from the file. I don't want to
>> mess too much with the logic in mov.c, that's why I provide both a
>> "memory" and a "file" input. Confusing, yes, slightly, but necessary as
>> long as you want a common function to be called from both sources. If
>> anyone else manages to come up with something better WITHOUT BREAKING
>> IT, no problem. It does take some knowledge about the structure of a
>> QuickTime video sample description.
>>
>> Mats
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> Actually I would prefer that nobody touches what I've been doing, since
> it works just fine right now, and it can be easily broken if you start
> trying to "improve it". Belive me, I've tried.
>
> Mats
>

Michael, will this pass as doxy documentation for ff_get_qtpalette()?

/**
  * Retrieve the palette (or "color table" in QuickTime terms), either
  * from the video sample description, or from the default Macintosh
  * palette.
  */


Mats

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