Justin Ruggles | 1 Oct 2007 02:52

Re: [PATCH] read metadata in FLAC demuxer

Rich Felker wrote:
> On Sun, Sep 30, 2007 at 02:47:18PM -0400, Justin Ruggles wrote:
>> Michael Niedermayer wrote:
>>> On Sun, Sep 30, 2007 at 12:48:08AM -0400, Justin Ruggles wrote:
>>>> Hi,
>>>>
>>>> Here is a patch to fix issue 187 in the FFmpeg Issue Tracker, "flac decoder 
>>>> fails with large metadata".
>>>>
>>>> For raw FLAC files, the metadata header(s) are read in the demuxer rather 
>>>> than the decoder.  This correctly skips any irrelevant metadata blocks, and 
>>>> it also parses vorbiscomment info.
>>> what happens with flac in avi and matroska ?
>> True, it should be handled in those demuxers as well.  And I do see your 
>> point as far as code duplication.  I'm working on a better solution with 
>> shared code between the decoder and all the demuxers.
> 
> For other formats, metadata should not be in Ogg-specific format but
> in whatever standard format the container uses.. I suspect that was
> Michael's point..

I see.  Well the streaminfo data should be in the same format no matter 
what the container.  Also, I believe a series of FLAC metadata blocks is 
standard for extradata in raw FLAC, Matroska, and other containers 
besides Ogg.  AFAIK, Ogg is the only one which uses its own unique 
layout for FLAC metadata.  The solution I'm working on takes this into 
account.

-Justin
(Continue reading)

Justin Ruggles | 1 Oct 2007 05:04

[PATCH] FLAC decoder segfault reading metadata

Hello,

While working on the other FLAC decoder bug, I found this similar one. 
Currently, the decoder reads past the end of the GetBitContext buffer if 
the metadata is cut off.  This can lead to a segfault or infinite loop.

I was able to trigger a segfault by creating a FLAC file with 2 large 
metadata blocks.  If you want to duplicate it, use:
flac test.wav -o test.flac
metaflac --dont-use-padding --remove-all test.flac
metaflac --add-padding=100000 test.flac
metaflac --add-padding=100000 test.flac

I've also uploaded a sample to MPlayer/incoming as black-2-pad.flac

$ gdb ./ffmpeg_g
GNU gdb 6.6-debian
Copyright (C) 2006 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain 
conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i486-linux-gnu"...
Using host libthread_db library "/lib/tls/i686/cmov/libthread_db.so.1".
(gdb) run -debug 1 -i black-2-pad.flac
Starting program: /home/justin/src/ffmpeg/ffmpeg_g -debug 1 -i 
black-2-pad.flac
FFmpeg version SVN-r10629, Copyright (c) 2000-2007 Fabrice Bellard, et al.
   configuration: --disable-ffserver --disable-vhook
(Continue reading)

Justin Ruggles | 1 Oct 2007 05:11

Re: [PATCH] FLAC decoder segfault reading metadata

Well...I thought I had looked this over long enough, but I guess not. 
Here is a better patch.

-Justin

Attachment (ffmpeg-flac-fix-meta.diff): text/x-patch, 2432 bytes
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel <at> mplayerhq.hu
http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
Rich Felker | 1 Oct 2007 05:18

Re: [PATCH] read metadata in FLAC demuxer

On Sun, Sep 30, 2007 at 08:52:35PM -0400, Justin Ruggles wrote:
> Rich Felker wrote:
> > On Sun, Sep 30, 2007 at 02:47:18PM -0400, Justin Ruggles wrote:
> >> Michael Niedermayer wrote:
> >>> On Sun, Sep 30, 2007 at 12:48:08AM -0400, Justin Ruggles wrote:
> >>>> Hi,
> >>>>
> >>>> Here is a patch to fix issue 187 in the FFmpeg Issue Tracker, "flac decoder 
> >>>> fails with large metadata".
> >>>>
> >>>> For raw FLAC files, the metadata header(s) are read in the demuxer rather 
> >>>> than the decoder.  This correctly skips any irrelevant metadata blocks, and 
> >>>> it also parses vorbiscomment info.
> >>> what happens with flac in avi and matroska ?
> >> True, it should be handled in those demuxers as well.  And I do see your 
> >> point as far as code duplication.  I'm working on a better solution with 
> >> shared code between the decoder and all the demuxers.
> > 
> > For other formats, metadata should not be in Ogg-specific format but
> > in whatever standard format the container uses.. I suspect that was
> > Michael's point..
> 
> I see.  Well the streaminfo data should be in the same format no matter 
> what the container.  Also, I believe a series of FLAC metadata blocks is 
> standard for extradata in raw FLAC, Matroska, and other containers 
> besides Ogg.  AFAIK, Ogg is the only one which uses its own unique 
> layout for FLAC metadata.  The solution I'm working on takes this into 
> account.

Matroska officially stores in-band FLAC-format metadata instead of
(Continue reading)

Justin Ruggles | 1 Oct 2007 06:16

Re: [PATCH] read metadata in FLAC demuxer

Rich Felker wrote:
> On Sun, Sep 30, 2007 at 08:52:35PM -0400, Justin Ruggles wrote:
>> Rich Felker wrote:
>>> On Sun, Sep 30, 2007 at 02:47:18PM -0400, Justin Ruggles wrote:
>>>> Michael Niedermayer wrote:
>>>>> On Sun, Sep 30, 2007 at 12:48:08AM -0400, Justin Ruggles wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Here is a patch to fix issue 187 in the FFmpeg Issue Tracker, "flac decoder 
>>>>>> fails with large metadata".
>>>>>>
>>>>>> For raw FLAC files, the metadata header(s) are read in the demuxer rather 
>>>>>> than the decoder.  This correctly skips any irrelevant metadata blocks, and 
>>>>>> it also parses vorbiscomment info.
>>>>> what happens with flac in avi and matroska ?
>>>> True, it should be handled in those demuxers as well.  And I do see your 
>>>> point as far as code duplication.  I'm working on a better solution with 
>>>> shared code between the decoder and all the demuxers.
>>> For other formats, metadata should not be in Ogg-specific format but
>>> in whatever standard format the container uses.. I suspect that was
>>> Michael's point..
>> I see.  Well the streaminfo data should be in the same format no matter 
>> what the container.  Also, I believe a series of FLAC metadata blocks is 
>> standard for extradata in raw FLAC, Matroska, and other containers 
>> besides Ogg.  AFAIK, Ogg is the only one which uses its own unique 
>> layout for FLAC metadata.  The solution I'm working on takes this into 
>> account.
> 
> Matroska officially stores in-band FLAC-format metadata instead of
> putting it in the Matroska headers where it belongs? I really doubt
(Continue reading)

Justin Ruggles | 1 Oct 2007 06:25

Re: [PATCH] read metadata in FLAC demuxer

Justin Ruggles wrote:
> Rich Felker wrote:
>> On Sun, Sep 30, 2007 at 08:52:35PM -0400, Justin Ruggles wrote:
>>> Rich Felker wrote:
>>>> On Sun, Sep 30, 2007 at 02:47:18PM -0400, Justin Ruggles wrote:
>>>>> Michael Niedermayer wrote:
>>>>>> On Sun, Sep 30, 2007 at 12:48:08AM -0400, Justin Ruggles wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Here is a patch to fix issue 187 in the FFmpeg Issue Tracker, "flac decoder 
>>>>>>> fails with large metadata".
>>>>>>>
>>>>>>> For raw FLAC files, the metadata header(s) are read in the demuxer rather 
>>>>>>> than the decoder.  This correctly skips any irrelevant metadata blocks, and 
>>>>>>> it also parses vorbiscomment info.
>>>>>> what happens with flac in avi and matroska ?
>>>>> True, it should be handled in those demuxers as well.  And I do see your 
>>>>> point as far as code duplication.  I'm working on a better solution with 
>>>>> shared code between the decoder and all the demuxers.
>>>> For other formats, metadata should not be in Ogg-specific format but
>>>> in whatever standard format the container uses.. I suspect that was
>>>> Michael's point..
>>> I see.  Well the streaminfo data should be in the same format no matter 
>>> what the container.  Also, I believe a series of FLAC metadata blocks is 
>>> standard for extradata in raw FLAC, Matroska, and other containers 
>>> besides Ogg.  AFAIK, Ogg is the only one which uses its own unique 
>>> layout for FLAC metadata.  The solution I'm working on takes this into 
>>> account.
>> Matroska officially stores in-band FLAC-format metadata instead of
>> putting it in the Matroska headers where it belongs? I really doubt
(Continue reading)

Loren Merritt | 1 Oct 2007 08:27

libavutil simd

Any objections to moving cputest.c and related code to libavutil? I'd like 
to simd some of the lavu functions, but cpu detection depends on lavc.

--Loren Merritt
Rich Felker | 1 Oct 2007 08:34

Re: [PATCH] read metadata in FLAC demuxer

On Mon, Oct 01, 2007 at 12:25:56AM -0400, Justin Ruggles wrote:
> Using mkvinfo, it appears that the metadata blocks are located in 
> CodecPrivate.

I suspect this was not intentional, but rather the result of copying a
flac stream into mkv without any special treatment for the metadata.
Thus it just got copied as-is along with the real CodecPrivate data
since they're stuck together in the source file. I doubt this practice
is encouraged by the Matroska developers and I wouldn't be surprised
if they discourage demuxers from reading this data, since it's in the
wrong place.. Someone authoring a proper MKV file should put this info
in the MKV metadata tags...

Rich
Michael Niedermayer | 1 Oct 2007 09:27
Picon
Picon

Re: libavutil simd

Hi

On Mon, Oct 01, 2007 at 12:27:12AM -0600, Loren Merritt wrote:
> Any objections to moving cputest.c and related code to libavutil? I'd like 
> to simd some of the lavu functions, but cpu detection depends on lavc.

cputest.c + the parts needed from the headers are ok

[...]
--

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- Måns Rullgård
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel <at> mplayerhq.hu
http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
Baptiste Coudurier | 1 Oct 2007 10:54
Favicon

Re: [PATCH] DNxHD/VC-3 encoder

Baptiste Coudurier wrote:
> Hi Michael,
> 
> Michael Niedermayer wrote:
>> Hi
>>
>> On Fri, Sep 07, 2007 at 08:05:28PM +0200, Michael Niedermayer wrote:
>>> Hi
>>>
>>> On Fri, Sep 07, 2007 at 09:17:49AM -0600, Loren Merritt wrote:
>>>> On Fri, 7 Sep 2007, Michael Niedermayer wrote:
>>>>
>>>>> when people start using qsort you know theres something wrong in their
>>>>> encoder (or at least not optimal)
>>>>>
>>>>> so, lets first check if i understand the constraints
>>>>> * its possible to change qscale per macroblock and it doesnt cost anything
>>>>> * theres a fixed number of bits available for each frame (pre mpeg1 design)
>>>>>
>>>>> in that case the optimal solution (though not fastest) is to do a
>>>>> binary search over lambda (using the number of bits and the ones available
>>>>> to find the best lambda which results in a frame which is not too large)
>>>>>
>>>>> then for each such lambda, find the best qscale for each MB, again by
>>>>> binary search minimizing SSD + bits*lambda (that way we know the best
>>>>> qscales and how many bits the frame would need)
>>>> Your algorithm produces the same decisions as the one with qsort. I don't 
>>>> know which is faster.
>>>>
>>>> Consider the list of possible block encodings sorted by 
(Continue reading)


Gmane