[FFmpeg-devel] [PATCH] FLAC parser

Michael Chinen mchinen
Wed Dec 8 01:03:00 CET 2010


On Tue, Dec 7, 2010 at 4:03 PM, Justin Ruggles <justin.ruggles at gmail.com> wrote:
> On 12/06/2010 07:43 PM, Alexander Strange wrote:
>
>>
>> On Dec 6, 2010, at 2:45 PM, Justin Ruggles wrote:
>>
>>> On 11/30/2010 09:28 PM, Michael Chinen wrote:
>>>
>>>> Hi,
>>>>
>>>> On Tue, Nov 30, 2010 at 1:59 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>>> On Wed, Nov 24, 2010 at 11:44:34PM +0100, Michael Chinen wrote:
>>>>>> On Tue, Nov 23, 2010 at 10:25 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>>>>> On Mon, Nov 22, 2010 at 05:19:23PM -0500, Justin Ruggles wrote:
>>>>> [...]
>>>>>>>> As for the AV_LOG_MAX patch, I think it's good but Michael N. needs to
>>>>>>>> ok it.
>>>>>>>
>>>>>>> probably a max of 127 should be more than enough
>>>>>>
>>>>>> Ok, here is the updated patches with comment to flac_read_fifo_wrap,
>>>>>> changelog, and minor version bump.
>>>>>>
>>>>>> Michael
>>>>>
>>>>> [...]
>>>>>> @@ -109,6 +109,12 @@ typedef struct {
>>>>>> #define AV_LOG_DEBUG ? ?48
>>>>>>
>>>>>> /**
>>>>>> + * The maximum AV_LOG value.
>>>>>> + * This can be useful as an offset to silence the log messages
>>>>>> + **/
>>>>>> +#define AV_LOG_MAX ? ? ?255
>>>>>
>>>>> thats not 127 but rethinking i see no need for this, offseting by appropriate
>>>>> values for the code used is better because with several levels of calls there can
>>>>> be no globally correct max/min offset
>>>>
>>>> Ok, here are the patches without AV_LOG_MAX.
>>>>
>>>
>>>> +static int frame_header_is_valid(AVCodecContext *avctx, const uint8_t *buf,
>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? FLACFrameInfo *fi)
>>>> +{
>>>> + ? ?GetBitContext gb;
>>>> + ? ?init_get_bits(&gb, buf, MAX_FRAME_HEADER_SIZE * 8);
>>>> + ? ?return !ff_flac_decode_frame_header(avctx, &gb, fi, AV_LOG_DEBUG);
>>>> +}
>>>
>>> I could be wrong, but I think that based on what Michael N. said, you
>>> should add 127 here instead of AV_LOG_DEBUG. ?The higher number is more
>>> appropriate for this code since you want it to always silence the output
>>> and you already know where and how the offset is being used.
>>>
>>> Other than that, patch set still looks good.
>>>
>>> -Justin
>>
>> I'd hate to see this thread end, but if you think it's OK, could you apply it with that change?
>
>
> All patches applied. ?I split the 3 patches into 7 commits to keep them
> more functionally separated.

Great.

>
> Thanks Michael for the patches and for your patience in getting them
> through review.

Thanks to you and everyone else for the thorough review, which I got a
lot out of.

Michael



More information about the ffmpeg-devel mailing list