Web lists-archives.com

Re: [MPlayer-dev-eng] [PATCH] mencoder: detect end of audio stream while encoded data still buffered




On Sun, May 4, 2014 at 11:55 PM, Reimar DÃffinger
<Reimar.Doeffinger@xxxxxx> wrote:
>
> Yes, after you pointed out how it works I think it kind of is.
> I do think it's less obvious than it should be though, and I am not sure it doesn't have issues like flushing the encoder buffer even on decode errors instead of EOF plus I suspect the same hang bug exists in certain -oac copy cases which this patch won't fix since it only affects the encoding case.

Okay, here is a second attempt at a patch. It's a bit longer this
time, but tidies up a few things, is more thorough about checking EOF
(at least as thorough as the rest of the code is) and fixes any issues
with other encode paths.

I have added a 'bytes_ready' variable so that there is less confusion
with the 'len' variable which is now just used for the decode length
not the encode length.

Patch description below.

---

Changes to mencoder introduced by r36905 (2014-02-24; mencoder: Finish
encoding only when audio and video reached EOF.) introduced a
regression when using mp3lame and possibly other audio encoders.

This was first reported by Alexander Roalter on mplayer-users:
https://lists.mplayerhq.hu/pipermail/mplayer-users/2014-March/087288.html

$ mencoder -msglevel all=6 -o out.avi -oac mp3lame -ovc lavc input
...
(works normally until the end of the audio stream, but after the end
of the video stream, an endless loop of messages begins:)
...
ds_fill_buffer: EOF reached (stream: audio)
ds_fill_buffer: EOF reached (stream: video)
ds_fill_buffer: EOF reached (stream: audio)
ds_fill_buffer: EOF reached (stream: video)
...

After running a debugger, I found the problem is that for some VBR
codecs like mp3lame, mux_a->buffer_len is not always zero on
end-of-stream.

This patch checks mux_a->buffer_len and ensures any remaining data is muxed.

This patch also adds a new variable 'bytes_ready' to avoid using the
same variable for both the number of decoded bytes AND encoded bytes.

Kieran Clancy

---

 mencoder.c |   41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)
Index: mencoder.c
===================================================================
--- mencoder.c	(revision 37183)
+++ mencoder.c	(working copy)
@@ -1310,7 +1310,8 @@
     // get audio:
     while(force_audio || a_muxer_time-audio_preload<v_muxer_time){
         float tottime;
-	int len=0;
+	int bytes_ready = 0;
+	int len;
 	force_audio = 0;
 
 	ptimer_start = GetTimerMS();
@@ -1342,9 +1343,9 @@
 			mux_a->buffer_len += aencoder->encode(aencoder, mux_a->buffer + mux_a->buffer_len,
 				aencoder->decode_buffer, len, mux_a->buffer_size-mux_a->buffer_len);
 			if(mux_a->buffer_len < mux_a->wf->nBlockAlign)
-				len = 0;
+				bytes_ready = 0;
 			else
-				len = mux_a->wf->nBlockAlign*(mux_a->buffer_len/mux_a->wf->nBlockAlign);
+				bytes_ready = mux_a->wf->nBlockAlign*(mux_a->buffer_len/mux_a->wf->nBlockAlign);
 		}
 		else	/* VBR */
 		{
@@ -1356,17 +1357,18 @@
 					sz = aencoder->get_frame_size(aencoder);
 				if(sz > 0 && mux_a->buffer_len >= sz)
 				{
-					len = sz;
+					bytes_ready = sz;
 					break;
 				}
 				len = dec_audio(sh_audio,aencoder->decode_buffer, aencoder->decode_buffer_size);
 				if(len <= 0)
 				{
-					len = 0;
+					/* EOF or error */
+					bytes_ready = 0;
 					break;
 				}
-				len = aencoder->encode(aencoder, mux_a->buffer + mux_a->buffer_len, aencoder->decode_buffer, len, mux_a->buffer_size-mux_a->buffer_len);
-				mux_a->buffer_len += len;
+				mux_a->buffer_len += aencoder->encode(aencoder, mux_a->buffer + mux_a->buffer_len,
+					aencoder->decode_buffer, len, mux_a->buffer_size-mux_a->buffer_len);
 			}
 	    }
 	    if (v_muxer_time == 0) mux_a->h.dwInitialFrames++;
@@ -1378,26 +1380,37 @@
 		len=mux_a->wf->nAvgBytesPerSec*tottime;
 		len/=mux_a->h.dwSampleSize;if(len<1) len=1;
 		len*=mux_a->h.dwSampleSize;
-		len=demux_read_data(sh_audio->ds,mux_a->buffer,len);
+		bytes_ready = demux_read_data(sh_audio->ds,mux_a->buffer,len);
 		break;
 	    }
 	} else {
 	    // VBR - encode/copy an audio frame
 	    switch(mux_a->codec){
 	    case ACODEC_COPY: // copy
-		len=ds_get_packet(sh_audio->ds,(unsigned char**) &mux_a->buffer);
+		bytes_ready = ds_get_packet(sh_audio->ds,(unsigned char**) &mux_a->buffer);
 		break;
 		}
 	    }
 	}
-	if(len<=0) { if (!mux_a->buffer_len && !sh_audio->a_out_buffer_len && sh_audio->ds->eof) at_eof |= 2; break; } // EOF?
-	muxer_write_chunk(mux_a,len,AVIIF_KEYFRAME, MP_NOPTS_VALUE, MP_NOPTS_VALUE);
+	if (bytes_ready<=0) {
+		/* check for EOS */
+		if (sh_audio->a_out_buffer_len || !sh_audio->ds->eof)
+			break; /* not EOS yet */
+
+		if (mux_a->buffer_len) {
+			bytes_ready = mux_a->buffer_len; /* still data to mux */
+		} else {
+			at_eof |= 2;
+			break;
+		}
+	}
+	muxer_write_chunk(mux_a,bytes_ready,AVIIF_KEYFRAME, MP_NOPTS_VALUE, MP_NOPTS_VALUE);
 	a_muxer_time = adjusted_muxer_time(mux_a); // update after muxing
 	if(!mux_a->h.dwSampleSize && a_muxer_time>0)
 	    mux_a->wf->nAvgBytesPerSec=0.5f+(double)mux_a->size/a_muxer_time; // avg bps (VBR)
-	if(mux_a->buffer_len>=len){
-	    mux_a->buffer_len-=len;
-	    memmove(mux_a->buffer,mux_a->buffer+len,mux_a->buffer_len);
+	if(mux_a->buffer_len>=bytes_ready){
+	    mux_a->buffer_len-=bytes_ready;
+	    memmove(mux_a->buffer,mux_a->buffer+bytes_ready,mux_a->buffer_len);
 	}
 
 
_______________________________________________
MPlayer-dev-eng mailing list
MPlayer-dev-eng@xxxxxxxxxxxx
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng