[MPlayer-dev-eng] [PATCH] audio file truncation

[Uoti Urpala]

On Mon, 2006-06-05 at 21:45 -0400, Alan Curry wrote:
> Here are 3 separate bugs with the same symptom: audio files are truncated
> when played by mplayer. It is most noticeable with low quality sound files

There's at least one clear bug in this patch, and there are also some
issues about what the resulting interface should be.

> +    if(sh_audio->a_in_buffer_len==0 && sh_audio->a_buffer_len==0)
> +      playflags |= AOPLAY_FINAL_CHUNK;

This is wrong. You can't detect the final chunk this way; it's normal
for those buffers to be empty in the middle of playback.

-  if(d_audio->eof && sh_audio->a_in_buffer_len <= 0 && sh_audio->a_buffer_len <= 0) eof = PT_NEXT_ENTRY;
+  if(d_audio->eof && sh_audio->a_in_buffer_len <= 0 && sh_audio->a_buffer_len <= 0 && sh_audio->a_out_buffer_len <= 0) eof = PT_NEXT_ENTRY;

While the change here isn't for the worse, the original and resulting
line are questionable. It's not impossible for the decoder to have some
internal buffering which cannot be seen from public buffer sizes. In my
opinion this code should not attempt to detect EOF before decode_audio()
returns <= 0.


> +  after the call is made. If flags & AOPLAY_FINAL_CHUNK, the end of the audio
> +  stream is being played, so it should try to play the full "len" amount.

AOPLAY_FINAL_CHUNK does not mean there's any more need to play the full
"len" amount than without the flag. What it does mean is that the ao
should not permanently reject the input because of its size; it's still
ok to buffer it only partially on this call. This does raise a question
though: if it's OK for the ao to play 0 bytes on this call, how do we
distinguish cases where the ao will accept data later (presumably after
some sound has played and buffers have emptied), and cases where it will
never succeed in playing the remaining bytes (assuming this is a
significant possibility)? Maybe the most reasonable thing to do is to
require that the ao accept at least some of the data if it returned > 0
from get_space().