[FFmpeg-devel] [PATCH] FLAC parser

[Justin Ruggles]

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.

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

Cheers,
Justin