[Ffmpeg-devel] [RFC] Real demuxer and audio reordering

[Rich Felker]

On Sun, Nov 20, 2005 at 06:00:29PM +0100, Roberto Togni wrote:
> Hi all.
> 
> Attached you can find a patch to move audio data reordering in
> rm demuxer for cook and 28_8 decoders. It also fix the extradata
> problem by storing it with a fixed endian (i chose BE because all
> data in rm files is BE).

WTF? The extradata is raw binary data IDENTICAL TO HOWEVER IT'S STORED
IN THE FILE!! There is no endianness to it because it is not a
ready-to-read data structure but a serialized platform-independent
format.

> Pro: it works, it works also with binary codecs, the number of packes
> read from file and sent to the codec is the same (no idea if that's
> useful for some applications), the codec is called a fewer times with
> bigger blocks of data.
> 
> Cons: the codec needs to split the packets into subpackets of
> coded_framesize (and more in case of cook), and to do it it needs some
> data from the container like codec_framesize and/or sub_packet_size

This is wrong. Demuxer must output individual packets, not multiple
concatenated packets.

> *** How things could work
> The demuxer does buffering and reorder like above, but instead of
> sending out packet of size block_align, it sends out packet of size
> coded_framesize (or sub_packet_size for cook).
> 
> Pro: no splitting is done in the codec, no container data (except from
> regular codec extradata) needs to be passed to the codec; should work
> with binary codecs (unchecked)

If it doesn't work as-is with them, the wrappers for binary codecs
just neet to be fixed to re-pack the data in whatever format the
binary codecs expect.

> Cons: for some codec the coded_framesize is quite small, so the codec
> is called a lot of times; there are many output packets for every
> packet got from file; we must create timestamps for intermediate
> packets (but see below)

Doesn't matter if there are cons; this is the only correct way.

> *** Extradata
> Now we pass two kind of extradata to the codec: the fist part is made
> from container data, the second part (if exists) is the "good" codec
> extradata.
> 
> The endianness problem is easily fixed by forcing a known endianness to
> the extradata fields (BE is the obvious choice in this case, see above)
> 
> The amount of extra extradata that is needed by the codec depends on
> the packetization method, and on the level of compatibility we want to
> keep with the binary codecs.

This is all broken. You should only pass the actual real extradata,
not additional crap you add on. If the demuxer works in the natural
correct way this problem goes away entirely.

> *** Timestamps
> In all samples i tested, all packets in a block have the same
> timestamp. In the patch i store them anyway and put them on the
> reordered packets, but this is probably not needed.
> Is it ok to have many packets with the same timestamp? I don't see any
> problem in my test, and the current behaviour (without my patch) is the
> same.

No, it's not correct. The timestamp only applies to one of the packets
(the first? the last?) and should only be attached to that one. If the
calling program needs more timestamps it will have to use a framer to
build correct timestamps for all the missing ones, but the demuxer
should NEVER output incorrect timestamps!

> *** Final questions
> - Is anybody against moving reordering to the demuxer?

Hopefully not. It's the only correct way.

> - Does anybody (outside ffmpeg/ffplay) uses lavf rm demuxer?

I dunno.

> - The change breaks compatibility for 28_8 codec, any complaint about
> it? Is anybody using it now (MPlayer have its own copy because of
> block size incompatibilities)

> - Do we care about being able to use binary codecs with lavf demuxer?

Yes and no. Yes, we should be able to use them, but it's up to the
wrapper code for the binary codec/loader to adapt the data back into
whatever broken/container-incest format the binary codec needs.

Rich