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

[Reimar Döffinger]

On Sun, May 04, 2014 at 05:19:10AM +0930, Kieran Clancy wrote:
> On Sun, May 4, 2014 at 4:31 AM, Reimar Döffinger
> <Reimar.Doeffinger at gmx.de> wrote:
> > I still think you are looking at the completely wrong thing.
> > Yes, the encoder can't flush data, so some audio data will be lost.
> > However the data buffer_len refers to is _already encoded_ data.
> 
> You are right, that sounds reasonable.
> 
> > Since you talk about "whole frame" you seem to (I believe mistakenly?)
> > assume that frames have some fixed size?
> > Whenever we get data back from the encoder it should be a frame.
> > Which might mean the fact that buffer_len becomes > 0 is a bug in the
> > first place.
> 
> I'm aware that frames have variable sizes.
> 
> Perhaps there are several problems then.
> 
> After the last encoded data is produced
> 
> len = aencoder->encode(...);
> 
> is giving > 0 bytes, but then the while(1) loop continues.
> 
> Then, on a subsequent loop,
> 
> len = dec_audio(...);
> 
> is returning 0, overwriting the previous non-zero value of len, and
> preventing muxer_write_chunk(...) on line 1394 from ever seeing the
> data.
> 
> Could the solution be as simple as something like this?
> 
> Index: mencoder.c
> ===================================================================
> --- mencoder.c    (revision 37181)
> +++ mencoder.c    (working copy)
> @@ -1362,7 +1362,7 @@
>                  len = dec_audio(sh_audio,aencoder->decode_buffer,
> aencoder->decode_buffer_size);
>                  if(len <= 0)
>                  {
> -                    len = 0;
> +                    len = mux_a->buffer_len;
>                      break;
>                  }
>                  len = aencoder->encode(aencoder, mux_a->buffer +
> mux_a->buffer_len, aencoder->decode_buffer, len,
> mux_a->buffer_size-mux_a->buffer_len);

That doesn't make sense, len is the amount of data to _encode_ here,
so it needs to be the amount of data in aencoder->decode_buffer.
But mux_a->buffer_len is the amount of already encoded data.
If we ignore the fact that we lose some amount of audio data at the end
for now, the only issue is that not all data is _muxed_.
There is no problem with the encoder or the amount of data it creates,
gets or anything at all.
I think your original patch was close to right, I just think it should
call the muxing function one last time with whatever remains in
mux_a->buffer_len (if anything).
(That is, unless you want to fix all the other issues in the code
as well, but one step at a time).