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

[Mats Peterson]

On 12/27/2015 10:38 AM, Mats Peterson wrote:
> On 12/27/2015 10:30 AM, Hendrik Leppkes wrote:
>> On Sun, Dec 27, 2015 at 4:42 AM, Mats Peterson
>> <matsp888-at-yahoo.com at ffmpeg.org> 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.
>>>
>>
>> And we would prefer if code is actually clean and not a convoluted
>> mess, and if you don't want us improving it, and you don't want to do
>> it yourself, then thats too bad.
>>
>> - Hendrik
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
> It's not a convoluted mess. It's better to have one function that is
> called by two different demuxers than duplicating code. And since it
> works well right now, why change it? We will see what Michael
> Niedermayer says about this. After all, he is the maintainer.
>
> Mats
>

Read the explanation above thoroughly, and you will hopefully understand 
the problem.

Mats

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