[FFmpeg-devel] [PATCH] FLAC parser

[Justin Ruggles]

Michael Chinen wrote:

> Hi,
> 
> On Thu, Nov 11, 2010 at 1:05 AM, Justin Ruggles
> <justin.ruggles at gmail.com> wrote:
>> Hi,
>>
>> Michael Chinen wrote:
>>
>>> On Sun, Oct 24, 2010 at 3:27 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>> On Sat, Oct 23, 2010 at 04:08:59AM +0200, Michael Chinen wrote:
>>>> [...]
>>>>> +static int find_new_headers(FLACParseContext *fpc, int search_start)
>>>>> +{
>>>>> +    FLACHeaderMarker *end;
>>>>> +    int search_end, size = 0, read_len, temp;
>>>>> +    uint8_t *buf;
>>>>> +    fpc->nb_headers_found = 0;
>>>>> +
>>>>> +    /* Search for a new header of at most 16 bytes. */
>>>>> +    search_end = av_fifo_size(fpc->fifo_buf) - (MAX_FRAME_HEADER_SIZE - 1);
>>>>> +    read_len   = search_end - search_start + 1;
>>>>> +    buf        = flac_fifo_read(fpc, search_start, &read_len);
>>>>> +    size       = find_headers_search(fpc, buf, read_len, search_start);
>>>>> +    search_start += read_len - 1;
>>>>> +
>>>>> +    /* if we hit the fifo end we need a wrap buffer for the two byte wrap around */
>>>>> +    if (search_start != search_end) {
>>>>> +        buf  = flac_fifo_read_wrap(fpc, search_start, 2,
>>>>> +                                  &fpc->wrap_buf, &fpc->wrap_buf_allocated_size);
>>>>> +        temp = find_headers_search(fpc, buf, 2, search_start);
>>>>> +        size = FFMAX(size, temp);
>>>>> +        search_start += 1;
>>>>> +    }
>>>> the 2 bytes can be checked directly and find_headers_search_validate() be called
>>> ok, I added this.
>>>
>>>> also i leave further review to the flac maintainer
>>>>
>>>> a bit of further simplification and cleanup would be nice though
>>> Also removed the error codes and used offsets for logs where applicable.
>> I think maybe AV_LOG_MAX should be added, which would match the highest
>> log level in case more levels above AV_LOG_DEBUG are added.  Otherwise
>> your code would stop working as intended.
> 
> I can't find a reference to AV_LOG_MAX with grep and AV_LOG_DEBUG is
> the largest value in log.h.

Hence, "AV_LOG_MAX should be added".  That's my suggested solution anyway.

-Justin