[FFmpeg-devel] [PATCH] HE-AAC v1 decoder try 4

[Michael Niedermayer]

On Fri, Feb 26, 2010 at 03:11:17PM -0500, Alex Converse wrote:
> On Fri, Feb 26, 2010 at 1:46 PM, Alex Converse <alex.converse at gmail.com> wrote:
> > On Fri, Feb 26, 2010 at 9:38 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> On Fri, Feb 26, 2010 at 03:29:41PM +0100, Michael Niedermayer wrote:
> >>> On Fri, Feb 26, 2010 at 12:36:53AM -0500, Alex Converse wrote:
> >>> > On Wed, Feb 24, 2010 at 11:06 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >>> > >
> >>> > > On Wed, Feb 24, 2010 at 10:31:43PM -0500, Alex Converse wrote:
> >>> > > > On Wed, Feb 24, 2010 at 9:20 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >>> > > > > On Wed, Feb 24, 2010 at 07:36:53PM -0500, Alex Converse wrote:
> >>> > > > >> On Tue, Feb 16, 2010 at 6:19 PM, Alex Converse <alex.converse at gmail.com> wrote:
> >>> > > > >> > On Tue, Feb 16, 2010 at 6:01 PM, Alex Converse <alex.converse at gmail.com>
> >>> > > > >> > wrote:
> >>> > > > >> >>
> >>> > > > >> >> Notes:
> >>> > > > >> >> *All the computation time is spent in ff_sbr_apply() and it's
> >>> > > > >> >> children. If it isn't called from ff_sbr_apply() making it 100% faster
> >>> > > > >> >> isn't going to buy us anything.
> >>> > > > >> >> *Right now the synthesis filterbank is written on top on an MDCT. With
> >>> > > > >> >> appropriate SIMD functions it may make sense to move it to an FFT.
> >>> > > > >> >> Right now the MDCT version is much faster.
> >>> > > > >> >> *SIMD placeholder patch not included
> >>> > > > >> >
> >>> > > > >> > Wrong patch.
> >>> > > > >> > try 4.1 :)
> >>> > > > >>
> >>> > > > >> Since the last patch I beat on the decoder with the zzuf hammer and
> >>> > > > >> fixed a few minor things as well as addressing the feeback I've seen
> >>> > > > >> in this thread.
> >>> > > > >>
> >>> > > > >> So I'm trying to figure out where we stand on this:
> >>> > > > >> *Does the ffplay issue block this?
> >>> > > > >
> >>> > > > > what ffplay issue?
> >>> > > > >
> >>> > > >
> >>> > > > The sample rate gets set late and ffplay winds up playing back at its
> >>> > > > default sample rate, as discovered by Vlad earlier in this thread.
> >>> > >
> >>> > > id say move the sample_rate=0 from the codec init to av_find_stream_info()
> >>> > > that is
> >>> > > if(aac) sample_rate=0; //elaborate comment
> >>> > >
> >>> > That still passes a zero to ffplay, unless you mean to do that before
> >>> > the has_codec_parameters() check
> >>>
> >>> i of course meant at the top of av_find_stream_info() not in the loop
> >>
> >> Thinking of this a little more, i think the sanest solution is to put the
> >> check in all affected demuxers. Anything else is a mess in terms of
> >> maintaince.
> >>
> >
> > Putting special logic in each demuxer for AAC seems liek a recipie for
> > disaster. We wind up with dozens of different test cases instead of
> > just a few.
> >
> >> which demuxers are affected by this at all?
> >
> > mp4, mov, mkv, raw (adts), wav
> > maybe caf, maybe flv
> >
> 
> On second thought less
> mkv, caf, avi don't set .frame_size for aac
> adts doesn't set .frame_size or .sample_rate
> 
>  But it still leaves a lot of configurations to check.
> 
> I think Vlad's (second) patch may be the cleanest solution. That or

the problem is that sample_rate is set wrong by lavf, its a bug in
lavf. This cannot be fixed by a change to one application usig lavf
(there are many many other applications that might need such a workaround
too -> not practical at all)


> having .init() set the frame_size to zero. 

Thats trading one wrong field against another wrong field, no solution


> Either of those would save
> a huge effort in checking each demuxer manually. Especially with the
> samples archive in such a sorry state.

then there is my first suggestion of setting sample_ratet to 0 for AAC
in av_find_stream_info()

The bug is that demuxers set sample_rate wrong, a fix should be in the
demuxers doing this or close to them.

we have a similar issue with width/height of h263 in mov that too is
set to 0 by the demuxer.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100226/7afce3e1/attachment.pgp>