[FFmpeg-devel] [PATCH] FLAC parser

[Justin Ruggles]

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.

-Justin