Fix Linux audio playback for short files

Fix Linux audio playback

Playing audio files shorter than about 11000 samples on Linux was either not working at all, or re-playing (starting and stopping on the same audio object) was working only intermittently.  It was possible to stop audio playback before the first buffer was played, resulting in no sound at all.  Also, if replaying, the buffer may have been dirty from the previous playback on the same device.

So on Linux at least, these issues appear fixed.

BUG=73045, 47761, 59367, 59369, 59370, 65618
TEST=Manual, quickly playing short audio files on Linux should work

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=81669

[Sergey Ulanov, 2011-03-31 22:04:46]

I'm concerned about clearing the buffer when we start new sound. Maybe it's
better to ensure that audio device is not reused for new sound until it's
finished playing the previous sound?

http://codereview.chromium.org/6776024/diff/1/media/audio/linux/alsa_output.cc
File media/audio/linux/alsa_output.cc (right):

http://codereview.chromium.org/6776024/diff/1/media/audio/linux/alsa_output.c...
media/audio/linux/alsa_output.cc:400: buffer_->Clear();
Does this mean we cut the sound that was playing before?

[scherkus (not reviewing), 2011-03-31 22:08:10]

nit but will defer to sergeyu for final review

http://codereview.chromium.org/6776024/diff/1/media/audio/linux/alsa_output.cc
File media/audio/linux/alsa_output.cc (right):

http://codereview.chromium.org/6776024/diff/1/media/audio/linux/alsa_output.c...
media/audio/linux/alsa_output.cc:590: GetCurrentDelay()) {
GetCurrentDelay() > 0 ?

[scherkus (not reviewing), 2011-04-05 17:39:11]

and updates here?

[davejcool, 2011-04-05 19:11:59]

These changes certainly helped the Linux desktop case, but did not help for
Chrome-OS or Mac, so I was looking into those cases first in case I found
anything else helpful.

On Tue, Apr 5, 2011 at 10:39 AM, <scherkus@chromium.org> wrote:

> and updates here?
>
>
> http://codereview.chromium.org/6776024/
>

[davejcool, 2011-04-09 03:10:42]

I needed to add a state check in StartTask(), WritePacket(), and WriteTask() so
that the function is only done if we're in the proper state.  Without this,
quickly restarting many short clips would start to not play the start or end of
them right.   With this, things play perfectly.

http://codereview.chromium.org/6776024/diff/1/media/audio/linux/alsa_output.cc
File media/audio/linux/alsa_output.cc (right):

http://codereview.chromium.org/6776024/diff/1/media/audio/linux/alsa_output.c...
media/audio/linux/alsa_output.cc:400: buffer_->Clear();
By this time, the previous stream's usage of this device has already been
stopped.  So if there's something left in the buffer, it's from the previous
stream being stopped with audio still in the buffer.  Without this clear, often
the start or end of a sound gets cut off.

http://codereview.chromium.org/6776024/diff/1/media/audio/linux/alsa_output.c...
media/audio/linux/alsa_output.cc:590: GetCurrentDelay()) {
On 2011/03/31 22:08:10, scherkus wrote:
> GetCurrentDelay() > 0 ?

Yes, >0 is better, thanks!

[scherkus (not reviewing), 2011-04-11 18:22:39]

tentative LGTM w/ one question

http://codereview.chromium.org/6776024/diff/10001/media/audio/linux/alsa_outp...
File media/audio/linux/alsa_output.cc (right):

http://codereview.chromium.org/6776024/diff/10001/media/audio/linux/alsa_outp...
media/audio/linux/alsa_output.cc:404: buffer_->Clear();
final question: are we sure this won't clobber any previous sound?

[davejcool, 2011-04-13 01:24:05]

I answered Andrew's question here about clearing buffer_ when starting.

http://codereview.chromium.org/6776024/diff/10001/media/audio/linux/alsa_outp...
File media/audio/linux/alsa_output.cc (right):

http://codereview.chromium.org/6776024/diff/10001/media/audio/linux/alsa_outp...
media/audio/linux/alsa_output.cc:404: buffer_->Clear();
On 2011/04/11 18:22:39, scherkus wrote:
> final question: are we sure this won't clobber any previous sound?

If there is something in buffer_, it must be from the previous usage of this
stream, and is invalid.  Only after starting is the first ScheduleNextWrite()
called, so the buffer_ can't have anything we need in it yet for the stream
currently starting, but it could have something leftover from the previous usage
of this stream.

At this time we expect the buffer to be zero length, if it's not, it will throw
off our computation of the hardware_delay in BufferPacket().

[scherkus (not reviewing), 2011-04-13 16:03:23]

thanks for clarifying -- makes sense

let's check it in!