[Ffmpeg-devel] [RFC] Musepack decoding support

[Michael Niedermayer]

Hi

On Wed, Dec 20, 2006 at 07:46:51AM +0200, Kostya wrote:
[...]
> > [...]
> > > +    get_buffer(&s->pb, buf, 4);
> > > +    t = LE_32(buf);
> > > +    samplerate = mpc_rate[(t >> 16) & 3];
> > > +    get_le32(&s->pb);
> > > +    get_le32(&s->pb);
> > > +    get_buffer(&s->pb, buf + 4, 4);
> > 
> > hmm thats a slightly odd way to read extradata, whats in the 2 32bit values
> > which are skiped? and why are they skiped?
> 
> Different peak values. 
> 

and why are they excluded with this complex code from extradata?


[...]
> [...]
> > uhm, reading through the whole file unconditionally during header parsing
> > is not ok, also this should be done in a generic way (like storing the
> > needed data in mpc_read_packet() and useing something like
> > av_build_index_raw() if the user wants
> 
> It's not that easy - frames make continious bitstream and mostly they won't end at byte
> boundaries (and bitstream is packed into 32-bit LE words read MSB)
> 
> What about giving error if url_is_streamed()!=0 (without those seeks the code will be a bit more
> complicated - we'll need to store additional 4 bytes of data for each frame).

hmm, ive no real objections, though it would be nice if url_is_streamed!=0
would be working too ...


> and index building may be skipped with flag AVFMT_FLAG_IGNIDX?

no, reading through the whole file by default is not acceptable, think of
a gb sized file ...

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus