[FFmpeg-devel] [PATCH] FLAC parser

[Michael Chinen]

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:
>> On 11/16/2010 08:50 AM, Michael Chinen wrote:
>>
>> > On Mon, Nov 15, 2010 at 10:57 PM, Justin Ruggles
>> > <justin.ruggles at gmail.com> wrote:
>> >> 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.
>> >
>> > Ah, I thought you meant 'add' as in offset.
>> >
>> > added as new 0001 patch.
>>
>>
>> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-log.h-add-AV_LOG_MAX.patch
Type: application/octet-stream
Size: 856 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101124/92e76752/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-move-decode_frame_header-from-flacdec.c-to-flac.c-h.patch
Type: application/octet-stream
Size: 8119 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101124/92e76752/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Add-error-codes-for-FLAC-header-parsing-and-move-log.patch
Type: application/octet-stream
Size: 5864 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101124/92e76752/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Add-FLAC-Parser.patch
Type: application/octet-stream
Size: 40174 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101124/92e76752/attachment-0003.obj>