On 06.05.2014, at 02:01, Kieran Clancy <clancy.kieran+mplayer@gmail.com> wrote:
On Tue, May 6, 2014 at 2:23 AM, Reimar Döffinger <Reimar.Doeffinger@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.