[MPlayer-dev-eng] NUT cleanup

[Rich Felker]

On Mon, Sep 05, 2005 at 01:42:38PM +0300, Oded Shimon wrote:
> > > 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

Nonsense! You have to construct the headers in memory before writing
them anyway, to avoid seeking. Just construct the rest, total the
size, then take ptr_size=log2(size)/7 (round up). Now take
ptr_size=log2(ptr_size+size)/7. I think this is safe; you might want
to check.

Actually, a common function could be made to do this -- assembling
whatever type of packet you want to make in memory, then finally
writing it with the pointers tacked on to the beginning once it's
finalized.

> 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.

You do have to count the whole packet size, yes. All this requires is
remembering the starting position...

> > > 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...

Don't exclude them, just make it possible for the caller to signal the
muxer that "next pts == INF" for some streams until another packet for
that stream becomes visible to the calling app. Alternatively the
caller should be able to signal any next pts it wants, not just INF,
to cause muxing to get stalled up for a while if it can't find the
next packet it needs for a given stream.

> > 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...

Yes, it must parse codec headers. Making any "nutmerge" tool that
generates blatently invalid files is not acceptable for the developers
to do, IMO. Surely some idiot will start using it...

If you won't make it parse codec headers, just make it refuse to mux
any codec that might contain B frames...

> 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...

Huh?

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

I prefer struct nut_context or struct NUTContext, in that order of
preference. I hate typedefs except for things like uint32_t. But it's
not really a big deal to me.

Anyway aren't the muxer and demuxer contexts totally different
entities? Or maybe not? It should probably be possible to separate the
code and compile with or without muxing/demuxing support for small
embedded applications.

Rich