[FFmpeg-devel] [RFC] Monkey's Audio decoder

[Michael Niedermayer]

Hi

On Wed, Sep 12, 2007 at 08:08:26AM +0300, Kostya wrote:
> On Tue, Sep 11, 2007 at 08:23:09PM +0200, Michael Niedermayer wrote:
> > Hi
> > 
> > On Tue, Sep 11, 2007 at 10:22:54AM +0300, Kostya wrote:
> > > On Tue, Sep 11, 2007 at 12:10:54AM +0200, Michael Niedermayer wrote:
> > > > Hi
> > > > 
> > > > On Mon, Sep 10, 2007 at 03:28:59PM +0300, Kostya wrote:
> > > > > Here's Monkey's Audio decoder made from libdemac by Benjamin Zores
> > > > > and made working by me.
> > > > > 
> > > > > Please comment on decoder/demuxer structure
> > > > > (ape.c is demuxer, apedec.c is decoder).
> > > > 
> > > > just a incomplete review as the code is too messy for my taste and this
> > > > wasnt a real submission but just RFC ...
> > > 
> > > Now here's an updated version with most issues addressed (aligning, comments,
> > > some functions, extradata is also in portable format).
> > > 
> > > And notes on design:
> > >   APE container resembles Musepack in the container design - data is stored
> > > in 32-bit little-endian words and there is no padding between frames, so
> > > in order to decode demuxer must read several bytes before actual start and
> > > send offset to decoder, which bswap()s input, skips tail of the previous frame
> > > and begins decoding.
> > >   Another distinct feature is that single frame may contain several megabytes
> > > of audio, which is clearly unusable. So this decoder (like demac does) decodes
> > > frame by 4608*channels samples. As it is impossible to detect their boundaries
> > > within frame data, demuxer sends first packet with frame data and then empty
> > > packets and decoder decodes only this fixed block of data per each call.
> > 
> > why does the code need these dummy packets? the application should call the
> > decoder again with not yet used data
> 
> a) data should be bswap32()d before use
> b) offset in data should be passed to decoder

i dont see how either of these would require the dummy packets
the decoder can bswap the data into an internal buffer (like it does currently
IIRC) and still correctly return the size of the decoded block
also it knows the offset of the next block from the end of the previous block
only the first offset of such a huge frame is unknown but that is passed anyway


> 
> While demuxer take care of this, it is hard to make application do so.
> Alternatives are handling the whole frames or decoding blocks to determine their
> boundaries (which requires half of decoder code duplication).

if its possible to split them efficiently (cpu time wise) that should be done
code duplication can be avoided by properly implementing it ...


> 
> The only problem with current approach - it makes error happening in decode_audio2()
> because input frame size is larger than output audio buffer (*frame_size_ptr < buf_size)
> but I am sure that condition may be dropped without any bad consequences.

well if you check all audio decoders for buffer overflows ...


> 
> > the problem with your approch is that if the stuff is stream copied then the
> > file will be full of these dummy packets
> 
> I can't imagine this situation - it's another case of codec/container lock in.

i can imaging people wanting to use it in avi, mkv, ...
and especially with mkv iam sure its only a matter of time until someone finds
a completely broken and sick way to do it (aka store the megabyte sized frames
as is) and without dummy packets ...

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- 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/20070912/9f7595ca/attachment.pgp>