[FFmpeg-devel] [PATCH] FLAC parser

[Michael Chinen]

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

Here is the latest set of patches.
I added a dependency on the FLAC parser for the flac demuxer - I don't
touch makefiles often so please check this.

Concerning hiding the patch history in a diffstat, I couldn't figure
out how to do this, so I just removed all that stuff since from
looking at other log messages I guess they should be summaries instead
of details.

Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-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/20101004/8963e2ac/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Add-FLAC-Parser.patch
Type: application/octet-stream
Size: 16117 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101004/8963e2ac/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-fix-ffplay-to-keep-calling-av_read_frame-even-if-EOF.patch
Type: application/octet-stream
Size: 709 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101004/8963e2ac/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-update-new-flac-seek-test-ref-file.patch
Type: application/octet-stream
Size: 4608 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101004/8963e2ac/attachment-0003.obj>