[MPlayer-dev-eng] NUT cleanup

[Oded Shimon]

On Mon, Sep 05, 2005 at 12:45:40AM -0400, Rich Felker wrote:
> On Sun, Sep 04, 2005 at 11:31:11PM +0300, Oded Shimon wrote:
> > 3. The spec doesn't say how you put the index "per stream", it only says to 
> > put a single index... Rich said entire index should be repeated for each 
> > stream, spec should mention this.
> 
> It's not repetition. The indices for each stream are presumably
> independent.

Yeah, that's what i meant.

> > 4. From where until where is the checksum calculated?... The spec doesn't 
> > say this either.
> 
> Which checksum? Anyway they're all a checksum of the whole NUT packet
> up to (but of course not including) the checksum itself.

Including the startcode?.. the current lavf/nut.c as far as i can tell does 
NOT include the startcode in the checksum.

> > 5. To extremely reduce both muxer and demuxer complexity, i suggest 
> > 'forward_ptr' should only be the length from right AFTER the vlc of the 
> > forward_ptr until the end of the checksum, NOT from the begginning of the 
> > startcode. Also I think the checksum should be only of this region too. 
> > This is not any less robust, it's just less of a pain in the ass to 
> > implement. Unless I'm missing something. (if your forward_ptr is broken, or 
> > your data is broken, or your crc is broken, you'll still get same results 
> > as the current behavior...)
> 
> Your implementation is broken if it's any more difficult to implement
> one way than the other..

Possibly, but it kind of a problem - forward_ptr includes it's own size! 
and since it's a VLC, by adding it's own size, it could grow! which is why 
I must:

1. in the muxer, add a 0x80 padding
2. in the demuxer, when reading it, I must remember/check its length - I 
had to re-implement get_v() privately for the packet header reader for 
this.

Granted, complexity is not THAT much worse, it's about 15 lines more code 
total in the muxer and demuxer. But is it a necessary complexity??.. Or am 
I still doing this broken and I should not be encountering this complexity?

I think this is very clean and neat:

// 'in' is a file buffer, 'out' is an empty memory buffer.
forward_ptr = get_v(in);
get_data(in, out, forward_ptr);
checksum = get_bytes(in, 4);
if (checksum != adler32(out)) {
	whatever;
}


I can't see any way to make it this simple in the way it is right now.

> > I've completed my own independant implementation of the muxer, it conforms, 
> > but it does no input error checking, and could be much smarter than it is 
> > now (better frame code tables etc.). I'm now working on atleast a working 
> > demuxer.
> 
> Last I heard, your muxer requires packets to already be in correct
> interleaved order.. We need a sorting input wrapper with dts
> calculation to make sure they're muxed correctly, and the unwrapped
> muxer needs to reject packets that are out of order.

Yes, you're right.. It's kind of one of my TODO's, to make a packet 
re-orderer, keeping a growing (big) buffer, excluding subtitles and 
metadata streams...

> > Alex has created a new CVS Repository, /cvsroot/nut with 'libnut' and 
> > 'nututils' direcotires. When I have a "barely complete" and working 
> > implementation of both muxer and demuxer, I'll commit. We are also waiting 
> > for Atilla to create dedicated mailing lists.
> 
> :)
> 
> > I am now looking for a brave individual that will start working on a 
> > 'nutmerge' util, that can atleast read an AVI file and use my NUT muxer API 
> > to create a nice conforming NUT file (for now, disregard B frames, we'll 
> > add that later too..).
> 
> Please DO NOT make a muxer that ignores B frames. If it doesn't
> support them it needs to check for a stream containing B frames and
> exit with fatal error if it finds one.

Well, what I mean is, atleast for now, I need some kind of AVI demuxer... 
finding B frames in an AVI is impossible without understanding the codec 
and reading the packets, right?... I want to save that complexity for now 
for atleast doing testing on my muxer...

> > Attached here is the nut.h API header, and an 
> > example muxer using /dev/urandom . Feel free to flame me to death on me 
> > making it a completely broken/stupid API. I've never written or even read a 
> > muxer implementation.
> 
> :)

I'm also having quite a bit of trouble regarding the demuxer API. I did 
something with stream input which I think is a good idea but Alex tells me 
it's a very bad idea:

typedef struct {
        void * priv;
        int (*seek)(void * priv, long pos, int dummy);
        size_t (*read)(uint8_t * buf, size_t dummy, size_t len, void * priv);
        off_t filesize;
} InputStreamContext;

I made it possible to use the FILE* API without changes. Alex tells me it's 
a bad idea cause I need to make a 'priv' and 'dummy' variables, but IMO 
without these, I'd need to make entire dummy _functions_, which are much 
more annoying...

BTW, a small religious vote, do you preffer 'NUTContext' or 
'nut_context_t'? :)

- ods15