[MPlayer-dev-eng] [PATCH] muxer frame buffering for mencoder [patch version 4]

[Nico Sabbi]

Corey Hickey wrote:

>This patch inserts a frame buffering function above the muxers as
>discussed on mplayer-advusers in the thread "[BUG] mencoder floating
>point exception with -of lavf" started by myself on 2005-11-04. Most of
>the reasoning behind this patch is in that thread, but I'll summarize here.
>
>Presently, mencoder attempts to write the header of the output file
>before knowing the characteristics of the video stream. This causes a
>floating point exception with the lavf muxer. The best solution offered
>to me was to write a buffering layer that stores frames until a video
>frame is seen, then write the header and send the stored frames to the
>actual muxer.
>
>The attached patch is the fourth version of my attempt at making this
>work. I have addressed most (if not all) of Nico's concerns, and this
>version should be at least close to finished. Significant differences
>from the previous patch:
>* frame data that need to be stored are now in a separate structure
>* there is no hard limit to the number of frames that can be buffered
>* frame storage is realloc()ed instead of being statically allocated
>* frames are buffered until at least one frame from each stream is seen
>* cleaner, I think
>
>  
>
[...]

>muxer_mpeg: The mpeg muxer didn't update the timer when passed an empty
>frame; my buffering function updates the timer regardless. As far as I
>know the behavior of my patched version is more correct. Files that
>encoded/played correctly with the unpatched version still seem to
>encode/play.
>  
>

in all my tests I didn't find a single case when s->buffer was null,
but anyway......

>------------------------------------------------------------------------
>
>Index: libmpdemux/muxer.c
>===================================================================
>RCS file: /cvsroot/mplayer/main/libmpdemux/muxer.c,v
>retrieving revision 1.10
>diff -u -r1.10 muxer.c
>--- libmpdemux/muxer.c	18 Nov 2005 14:39:19 -0000	1.10
>+++ libmpdemux/muxer.c	19 Nov 2005 06:22:27 -0000
>@@ -48,3 +48,100 @@
>     }
>     return muxer;
> }
>+
>+/* buffer frames until we either:
>+ * (a) have at least one frame from each stream
>+ * (b) run out of memory */
>+void muxer_write_chunk(muxer_stream_t *s, size_t len, unsigned int flags) {
>+    if (s->muxer->muxbuf_skip_buffer) {
>+      s->muxer->cont_write_chunk(s, len, flags);
>+    }
>+    else {
>+      int num = s->muxer->muxbuf_num++;
>+      muxbuf_t *buf;
>+      
>+      s->muxer->muxbuf = realloc(s->muxer->muxbuf, (num+1) * sizeof(muxbuf_t));
>  
>

this is a possible memleak: if s->muxer->muxbuf != NULL before realloc()
but realloc() fails you will lose track of the previously allocated buffer.
Don't bother fixing it, I will do it myself if my tests succeed


>Index: libmpdemux/muxer_mpeg.c
>===================================================================
>RCS file: /cvsroot/mplayer/main/libmpdemux/muxer_mpeg.c,v
>retrieving revision 1.22
>diff -u -r1.22 muxer_mpeg.c
>--- libmpdemux/muxer_mpeg.c	18 Nov 2005 14:39:19 -0000	1.22
>+++ libmpdemux/muxer_mpeg.c	19 Nov 2005 06:22:27 -0000
>@@ -1991,7 +1991,6 @@
> 
> 		if(s->buffer[3])
> 		{	// Sequence or GOP -- scan for Picture
>-			s->gop_start = s->h.dwLength;
> 			while (ptr < len-5 && 
> 				(s->buffer[ptr] != 0 || s->buffer[ptr+1] != 0 || s->buffer[ptr+2] != 1 || s->buffer[ptr+3] != 0)) 
> 				ptr++;
>@@ -2296,7 +2295,7 @@
> 
> extern int aac_parse_frame(uint8_t *buf, int *srate, int *num);
> 
>-static int parse_audio(muxer_stream_t *s, int finalize, int *nf, double *timer)
>+static int parse_audio(muxer_stream_t *s, int finalize)
>  
>

I don't like this. Don't remove useful infos like those: they may be 
needed in the future.
The rest looks ok to me, at least reading the code. If my tests succeed 
and no one complains
I'll commit your patch in the next days.

Thanks,
    Nico