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

[Reimar Döffinger]

On 06.05.2014, at 02:01, Kieran Clancy <clancy.kieran+mplayer at gmail.com> wrote:
> On Tue, May 6, 2014 at 2:23 AM, Reimar Döffinger
> <Reimar.Doeffinger at gmx.de> wrote:
>> 
>> What exactly do you mean by "for some"?
>> For mp3lame it is this crappy workaround in ae_lame.c that should cause it:
>> mux_a->wf->nBlockAlign=encoder->params.samples_per_frame; // required for l3codeca.acm + WMP 6.4
>> 
>> I would hope we have no such hacks for all others.
> 
> How does that line cause the problem? I'm not really sure what it does.
> 
> It may well be in practice that mp3lame is the only codec affected. In
> theory though, I don't see how a VBR codec can be expected to guess in
> advance exactly how long its audio frame will be?

It can't but this line above claims that all audio frames are exactly samples_per_frame long.
I am not completely sure it is this line though, it might be that the get_frame_size function instead returns something wrong.

>>> This patch checks mux_a->buffer_len and ensures any remaining data is muxed.
>> 
>> Any reason to not just do it like attached patch?
> 
> It makes more sense to me to do it inside the audio loop than as an
> addition at the end (ideally neither video nor audio frames would
> require flushing after their respective loops).

Having it in the middle of the audio is in fact what I don't like, because it makes it more complex (though if I filter out the renaming I have to admit not by that much), and I think it is already more complex than it should be in the first place.
Keep in mind that this is a special case for when
a) we reach EOS and
b) there is a buggy encoder or other issue that causes us to have data left over

b) is also the reason why I think it should trigger a warning.

> If there's something you don't like about the coding style in my
> patch, feel free to change it, but I think it's the right place to put
> it.

I'd at least want to split the renaming (maybe something that should be done regardless), though bytes_ready seems to generic as well, bytes_to_mux maybe is better?
Maybe after that it will look simple enough to not make it worth putting it outside the loop.