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

[Kieran Clancy]

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

When I was debugging the problem, mp3lame was returning many different
frame sizes, it wasn't fixed. Is it possible that the frame size is
not "wrong" but just LAME's best guess based on previous data?

I just checked; ae_lame.c's get_frame_size is more-or-less just:

mp_decode_mp3_header(encoder->stream->buffer);

All this function does is calculate frame size from the current VBR
bitrate. So LAME is choosing a bitrate based on the first chunk of
audio data it receives and then get_frame_size is calculating the
expected frame size. Looking at the MP3 file spec, this seems
reasonable.

In fact, I believe this means my first intuition was probably correct
after all; this means the data in a_mux->buffer is from a incomplete
frame and should not be written at all!

Sure enough, when I do encodes with or without the extra bytes:

mencoder summary, with first patch:
Video stream: 13982.967 kbit/s  (1747870 B/s)  size: 8675193 bytes
4.963 secs  123 frames
Audio stream:  162.319 kbit/s  (20289 B/s)  size: 100800 bytes  4.968 secs

mencoder summary, with latest patch:
Video stream: 13982.967 kbit/s  (1747870 B/s)  size: 8675193 bytes
4.963 secs  123 frames
Audio stream:  162.152 kbit/s  (20269 B/s)  size: 101183 bytes  4.992 secs

So we have 383 more bytes in the second audio stream, apparently.

But, ffmpeg complains when processing the second file:

[mp3 @ 0x150b300] incomplete frame
Error while decoding stream #0:1: Invalid data found when processing input

Unsurprisingly, libmpcodec's twolame and toolame implementations have
the same problem.

Naturally I decided to look at how lavc does things. It seems that
lavc only even returns whole frames, never partial frames (so it
doesn't really use the mux_a buffer "concept"). Its get_frame_size the
exact size of the frame if it's ready (and 0 otherwise). I couldn't
tell but I guess it has an internal buffer in which it stores excess
encoded data for using in the next frame?

So to me it seems the best solution, short of a massive rewrite, is to
just remove the check on buffer_len exactly like my first patch does,
and silently discard incomplete audio frames as has been done since
the dawn of mencoder (I guess). You could put a debug level notice.

As I said in my first email, it would be nice if mencoder had the
architecture to request encoders complete their frames on EOS (both
LAME and lavc have capabilities for padding the final frame
appropriately, though this can result in slightly longer audio than
the original file), but it would require big changes.

Kieran