[FFmpeg-devel] [PATCH] FLAC parser

[Justin Ruggles]

Michael Chinen wrote:

> On Sat, Oct 2, 2010 at 10:40 PM, Justin Ruggles
> <justin.ruggles at gmail.com> wrote:
>> Justin Ruggles wrote:
>>
>>> Michael Chinen wrote:
>>>
>>>> On Wed, Sep 29, 2010 at 2:51 AM, Justin Ruggles
>>>> <justin.ruggles at gmail.com> wrote:
>>>>> Justin Ruggles wrote:
>>>>>
>>>>>> Michael Chinen wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Mon, Sep 27, 2010 at 1:32 AM, Felipe Contreras
>>>>>>> <felipe.contreras at gmail.com> wrote:
>>>>>>>> 2010/9/26 M?ns Rullg?rd <mans at mansr.com>:
>>>>>>>>> Felipe Contreras <felipe.contreras at gmail.com> writes:
>>>>>>>>>
>>>>>>>>>> On Sun, Sep 26, 2010 at 1:58 PM, Michael Chinen <mchinen at gmail.com> wrote:
>>>>>>>>>>> On Sun, Sep 26, 2010 at 2:00 AM, Felipe Contreras
>>>>>>>>>>> [...]
>>>>>>>>>>>> Ah, I needed CONFIG_FLAC_PARSER.
>>>>>>>>>>> I would like to test this.  I am running configure without any mention
>>>>>>>>>>> of the parser.  How can I get it to go without CONFIG_FLAC_PARSER?
>>>>>>>>>> I guess --disable-parser=flac
>>>>>>>>>>
>>>>>>>>>>>> Since now the decoder doesn't work without the parser, wouldn't a
>>>>>>>>>>>> dependency should somehow be specified?
>>>>>>>>>>> Is this dependency undesirable?  I can look into getting both cases to
>>>>>>>>>>> work if so.
>>>>>>>>>> I don't know, but with your patches this stopped working:
>>>>>>>>>> ./configure --disable-everything --enable-decoder='flac'
>>>>>>>>>> --enable-demuxer='ogg,flac' --enable-protocol=file
>>>>>>>>>> --enable-encoder='pcm_s16le' --enable-muxer='wav'
>>>>>>>>>>
>>>>>>>>>> ffmpeg -i test.flac test.wav
>>>>>>>>> The demuxer probably needs the parser to find frame boundaries.  Can
>>>>>>>>> it play flac in ogg (where ogg provides the framing)?
>>>>>>>> I haven't tried this yet, but at least it doesn't seem to work with my
>>>>>>>> gst-av[1] stuff, so I think the problem is in the decoder itself.
>>>>>>> Any ogg related thing is definitely not supported in this
>>>>>>> parser/decoder (nor in the previous flacdec.c).
>>>>>>> flac in ogg is .ogg file right? - should it be handled by this parser/decoder?
>>>>>>>
>>>>>>> The patch isn't ready quite yet, but I'm working on it.
>>>>>> I can confirm that Ogg/FLAC doesn't work when you disable the parser,
>>>>>> but it does work when you enable the parser.  This is weird because the
>>>>>> Ogg demuxer doesn't even set st->need_parsing.  Plus, like Mans said,
>>>>>> Ogg shouldn't need to use the parser.  If Michael doesn't already have
>>>>>> it figured out, I'll see if I can find the problem this afternoon after
>>>>>> work.
>>>>> Ok, I take that back.  I thought I was testing an ogg file, but it was
>>>>> really a raw flac file.  I tested again with ogg/flac files made with
>>>>> both the flac commandline tool and with ffmpeg and the previous patch,
>>>>> including changes to flacdec.c work just fine for me.  Can anyone else
>>>>> reproduce the problems that Felipe is having?  Or can you give more info
>>>>> Felipe?
>>>> When I configure with --disable-parser=flac
>>>>
>>>> if I run files generated with "ffmpeg -i a.wav -acodec flac a.ogg"
>>>> The decoder works fine.
>>>>
>>>> but if I try to do Yesterday.ogg (flac in ogg sample) it fails
>>>>
>>>> I get a fail:
>>>> [NULL @ 0x1002600] Probed with size=2048 and score=100
>>>> [flac @ 0x1004000] invalid frame header: frame sync error
>>>> [flac @ 0x1004000] decode_frame() failed
>>>> [flac @ 0x1004000] invalid frame header: frame sync error
>>>> [flac @ 0x1004000] decode_frame() failed
>>>> [flac @ 0x1004000] invalid frame header: frame sync error
>>>> [flac @ 0x1004000] decode_frame() failed
>>>> (etc)
>>> Yesterday.ogg is an "old style" (pre libFLAC 1.1.1) Ogg/FLAC file.  To
>>> quote the "ogg mapping" page on the FLAC website:
>>>
>>> "It should also be noted that support for encapsulating FLAC in Ogg has
>>> been present in the FLAC tools since version 1.0.1. However, the
>>> mappings used were never formalized and have insurmountable problems.
>>> For that reason, Ogg FLAC streams created with flac versions before
>>> 1.1.1 should be decoded and re-encoded with flac 1.1.1 or later (flac
>>> 1.1.1 can decode all previous Ogg FLAC files, but files made prior to
>>> 1.1.0 don't support seeking). Since the support for Ogg FLAC before FLAC
>>> 1.1.1 was limited, we hope this will not result in too much inconvenience."
>>>
>>>> metaflac --list ~/Desktop/Yesterday.ogg
>>> ...
>>>   vendor string: reference libFLAC 1.1.0 20030126
>>>
>>> We should still try to support those old files, but that is a completely
>>> different issue I think.  What I get is a few frame sync errors, then a
>>> bunch of errors that the channel layout cannot change mid-stream.  So
>>> the first few packets are likely header packets that are not identified
>>> as such.  The remaining packets seem correct.  That shouldn't be too
>>> hard to work around.
>> I just committed a fix to the FLAC decoder that should allow it to work
>> with this file after the FLAC parser is added and the current decoder
>> buffering hack is removed.
>>
> 
> Thanks, after your work I verified that "ffmpeg -i Yesterday.ogg
> a.wav" works without the parser, in that it outputs a valid wav file,
> but I get a few error messages that weren't there before.  But maybe
> these are expected given the issues you mention with the legacy nature
> of this file?
> This happens three times:
> [flac @ 0x1004000] invalid frame header: frame sync error
> [flac @ 0x1004000] decode_frame() failed

Handling raw headers in the decoder is one thing, but I don't think it's
a good idea for the decoder to try to process Ogg-specific headers.  I
haven't looked into it in-depth yet to see why the demuxer doesn't
handle those first few packets properly.

-Justin