[FFmpeg-devel] [PATCH] FLAC parser

[Justin Ruggles]

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