Opened 13 years ago

Last modified 12 years ago

#754 reopened defect

Audio device is not flushed on close

Reported by: svs@… Owned by: reimar
Priority: if idle Component: ao
Version: HEAD Severity: normal
Keywords: Cc:
Blocked By: Blocking:
Reproduced by developer: Analyzed by developer:

Description

ao_sun has code to flush audio device, but only for SunOS, not NetBSD.

Attachments (3)

ao_sun.patch (353 bytes) - added by svs@… 13 years ago.
proposed patch
patch-stutter (481 bytes) - added by svs@… 12 years ago.
stop stuttering after resuming playback
754-2.diff (881 bytes) - added by svs@… 12 years ago.
proposed fix

Download all attachments as: .zip

Change History (14)

Changed 13 years ago by svs@…

proposed patch

comment:1 Changed 13 years ago by svs@…

Changed 12 years ago by svs@…

stop stuttering after resuming playback

comment:2 Changed 12 years ago by svs@…

comment:3 Changed 12 years ago by reimar

  • Resolution set to fixed
  • Status changed from new to closed

Fixed first issue in r23352 with a different patch (tested on OpenBSD).
The second patch should have gone in a extra bugreport and looks extremely like a hack, the reason for those problem should be found first. I am not sure if I can reproduce it either, is the stuttering very slight or strong for you?

comment:4 Changed 12 years ago by svs@…

Yes, stuttering is strong. It goes away after seeking forward or back, so I guessed that sample counter should be reset after resuming.

comment:5 Changed 12 years ago by reimar

Well, the stuttering problem is not there with OpenBSD, it only happens with NetBSD, so maybe a driver bug? Either way, your patch does not fix it for me so the problem is elsewhere anyway.

comment:6 Changed 12 years ago by svs@…

I'll try to track down the root cause then...

comment:7 Changed 12 years ago by svs@…

  • Resolution fixed deleted
  • Status changed from closed to reopened

Fixed first issue in r23352 with a different patch (tested on OpenBSD).

There are problems still:

  • the test for 'immed' in uninit() is reversed -- if immed==1, we shouldn't flush audio;
  • 'flush' should play remaining audio data (AUDIO_DRAIN), not discard them (AUDIO_FLUSH);
  • AUDIO_DRAIN is called for no reason -- in init() and reset(), immediately after open().

Changed 12 years ago by svs@…

proposed fix

comment:8 Changed 12 years ago by svs@…

  • attachments.isobsolete changed from 0 to 1

comment:9 Changed 12 years ago by svs@…

The second patch should have gone in a extra bugreport and looks extremely like
a hack, the reason for those problem should be found first.

This might be a kernel bug (shared by NetBSD and OpenBSD) -- http://arkiv.netbsd.se/?ml=openbsd-tech&a=2006-04&t=1974138

comment:10 Changed 12 years ago by reimar

  • the test for 'immed' in uninit() is reversed -- if immed==1, we shouldn't flush audio;
  • 'flush' should play remaining audio data (AUDIO_DRAIN), not discard them

(AUDIO_FLUSH);

Uh, no, the audio_flush function of course should flush the data, I sure hope you were not fully awake when you thought changing a function named audio_flush to do drain instead of flush was a good idea.
There _may_ be a ioctl(audio_fd, AUDIO_DRAIN, 0); missing in an else part of "if (immed)", though with other sound APIs like OSS the default behaviour is for the close to block, so it would be preferable to have some documentation on this.
The AUDIO_DRAIN in init does indeed seem pointless, whereas the one in reset() almost certainly should be audio_flush (i.e. discard previous audio data).

comment:11 Changed 12 years ago by svs@…

Uh, no, the audio_flush function of course should flush the data

Yeah, I was confused, sorry. For the record:

uninit() has two modes of operation -- immed=1 (discard audio data) and immed=0 (play audio data). Both BSD and Solaris do an implicit drain on close().

The AUDIO_DRAIN in init does indeed seem pointless, whereas the one in reset()
almost certainly should be audio_flush (i.e. discard previous audio data).

reset() calls AUDIO_DRAIN immediately after open() -- what's the point?

Note: See TracTickets for help on using tickets.