Hi,
On Sun, 2008-05-04 at 19:19 +0200, Mark Wielaard wrote:
> I had the same problem with a similar setup and with applications that
> forget to close a line they don't use anymore (unfortunately this seems
> very common). With the current directaudio backend I don't see how multiple
> lines for the same hardware device could work though. So I am using a
> trick to look for "sloppy" applications. If the last line opened in the
> directaudio device was for the same hardware format then we silence that
> one first so we can hand out a new one. This seems to work surprisingly
> well. And it doesn't seem to interfere with applications that handle the
> hardware formats they need explicitly.
>
> With gcjwebplugin and this patch we can happily play the vNES games :)
>
> Of course a real solution would be to import or write a better mixer
> that does share lines properly.
I updated the patch a little to make it all a bit more robust. It splits
the locks, so there is no longer one gaint static one (I shouldn't have
reused the lockNative one and made that static in the original patch).
And it makes sure that all methods that require the nativeLock check the
doIO inside their synchronized block and the lock around nStop() isn't
released before the flag is set. ALSA can actually hang when called on a
pcm you already stopped or closed. This race was already in the code,
since a DataSourceLine could be asynchronously stopped or closed at any
time. My patch just exposed it more easily, because DirectAudioDevice is
the mixer that is now always used by defailt.
2008-05-08 Mark Wielaard <[EMAIL PROTECTED]>
* patches/icedtea-directaudio-close-trick.patch: Use new static
lockLast for nOpen/nClose guarding. Make lockNative non-static
again. Do all checks before native calls of doIO inside
lockNative guard. Set doIO to false after nClose before dropping
lockNative guard.
I do think we should write a new MixerProvider based on a modern
soundserver, like pulseaudio. The current DirectAudioDeviceProvider code
cannot easily be extended to provide true software sound mixing. And the
way it opens direct hardware alsa devices means it locks out other
applications.
Cheers,
Mark
Patch against original code (updated patch file in icedtea6).
--- /home/mark/src/openjdk/jdk/src/share/classes/com/sun/media/sound/DirectAudioDevice.java 2008-04-13 01:05:30.000000000 +0200
+++ openjdk/jdk/src/share/classes/com/sun/media/sound/DirectAudioDevice.java 2008-05-09 02:18:21.000000000 +0200
@@ -394,7 +394,12 @@
private float leftGain, rightGain;
protected volatile boolean noService = false; // do not run the nService method
+ // Guards all native calls.
protected Object lockNative = new Object();
+ // Guards the lastOpened static variable in implOpen and implClose.
+ protected static Object lockLast = new Object();
+ // Keeps track of last opened line, see implOpen "trick".
+ protected static DirectDL lastOpened;
// CONSTRUCTOR
protected DirectDL(DataLine.Info info,
@@ -496,20 +501,47 @@
// align buffer to full frames
bufferSize = ((int) bufferSize / format.getFrameSize()) * format.getFrameSize();
- id = nOpen(mixerIndex, deviceID, isSource,
- encoding,
- hardwareFormat.getSampleRate(),
- hardwareFormat.getSampleSizeInBits(),
- hardwareFormat.getFrameSize(),
- hardwareFormat.getChannels(),
- hardwareFormat.getEncoding().equals(AudioFormat.Encoding.PCM_SIGNED),
- hardwareFormat.isBigEndian(),
- bufferSize);
+ synchronized(lockLast) {
+ id = nOpen(mixerIndex, deviceID, isSource,
+ encoding,
+ hardwareFormat.getSampleRate(),
+ hardwareFormat.getSampleSizeInBits(),
+ hardwareFormat.getFrameSize(),
+ hardwareFormat.getChannels(),
+ hardwareFormat.getEncoding().equals(AudioFormat.Encoding.PCM_SIGNED),
+ hardwareFormat.isBigEndian(),
+ bufferSize);
+
+ if (id == 0) {
+ // Bah... Dirty trick. The most likely cause is an application
+ // already having a line open for this particular hardware
+ // format and forgetting about it. If so, silently close that
+ // implementation and try again. Unfortuantely we can only
+ // open one line per hardware format currently.
+ if (lastOpened != null
+ && hardwareFormat.matches(lastOpened.hardwareFormat)) {
+ lastOpened.implClose();
+ lastOpened = null;
+
+ id = nOpen(mixerIndex, deviceID, isSource,
+ encoding,
+ hardwareFormat.getSampleRate(),
+ hardwareFormat.getSampleSizeInBits(),
+ hardwareFormat.getFrameSize(),
+ hardwareFormat.getChannels(),
+ hardwareFormat.getEncoding().equals(AudioFormat.Encoding.PCM_SIGNED),
+ hardwareFormat.isBigEndian(),
+ bufferSize);
+ }
+
+ if (id == 0) {
+ // TODO: nicer error messages...
+ throw new LineUnavailableException("line with format "+format+" not supported.");
+ }
+ }
+ lastOpened = this;
+ }
- if (id == 0) {
- // TODO: nicer error messages...
- throw new LineUnavailableException("line with format "+format+" not supported.");
- }
this.bufferSize = nGetBufferSize(id, isSource);
if (this.bufferSize < 1) {
// this is an error!
@@ -580,12 +612,12 @@
}
synchronized (lockNative) {
nStop(id, isSource);
- }
- // need to set doIO to false before notifying the
- // read/write thread, that's why isStartedRunning()
- // cannot be used
- doIO = false;
+ // need to set doIO to false before notifying the
+ // read/write thread, that's why isStartedRunning()
+ // cannot be used
+ doIO = false;
+ }
// wake up any waiting threads
synchronized(lock) {
lock.notifyAll();
@@ -614,8 +646,12 @@
doIO = false;
long oldID = id;
id = 0;
- synchronized (lockNative) {
- nClose(oldID, isSource);
+ synchronized (lockLast) {
+ synchronized (lockNative) {
+ nClose(oldID, isSource);
+ if (lastOpened == this)
+ lastOpened = null;
+ }
}
bytePosition = 0;
softwareConversionSize = 0;
@@ -630,7 +666,8 @@
}
int a = 0;
synchronized (lockNative) {
- a = nAvailable(id, isSource);
+ if (doIO)
+ a = nAvailable(id, isSource);
}
return a;
}
@@ -644,9 +681,9 @@
int counter = 0;
long startPos = getLongFramePosition();
boolean posChanged = false;
- while (!drained && doIO) {
+ while (!drained) {
synchronized (lockNative) {
- if ((id == 0) || !nIsStillDraining(id, isSource))
+ if ((id == 0) || (!doIO) || !nIsStillDraining(id, isSource))
break;
}
// check every now and then for a new position
@@ -686,7 +723,7 @@
lock.notifyAll();
}
synchronized (lockNative) {
- if (id != 0) {
+ if (id != 0 && doIO) {
// then flush native buffers
nFlush(id, isSource);
}
@@ -697,9 +734,10 @@
// replacement for getFramePosition (see AbstractDataLine)
public long getLongFramePosition() {
- long pos;
+ long pos = 0;
synchronized (lockNative) {
- pos = nGetBytePosition(id, isSource, bytePosition);
+ if (doIO)
+ pos = nGetBytePosition(id, isSource, bytePosition);
}
// hack because ALSA sometimes reports wrong framepos
if (pos < 0) {
@@ -745,11 +783,12 @@
}
int written = 0;
while (!flushing) {
- int thisWritten;
+ int thisWritten = 0;
synchronized (lockNative) {
- thisWritten = nWrite(id, b, off, len,
- softwareConversionSize,
- leftGain, rightGain);
+ if (doIO)
+ thisWritten = nWrite(id, b, off, len,
+ softwareConversionSize,
+ leftGain, rightGain);
if (thisWritten < 0) {
// error in native layer
break;
@@ -972,9 +1011,10 @@
}
int read = 0;
while (doIO && !flushing) {
- int thisRead;
+ int thisRead = 0;
synchronized (lockNative) {
- thisRead = nRead(id, b, off, len, softwareConversionSize);
+ if (doIO)
+ thisRead = nRead(id, b, off, len, softwareConversionSize);
if (thisRead < 0) {
// error in native layer
break;
@@ -1209,7 +1249,8 @@
// set new native position (if necessary)
// this must come after the flush!
synchronized (lockNative) {
- nSetBytePosition(id, isSource, frames * frameSize);
+ if (doIO)
+ nSetBytePosition(id, isSource, frames * frameSize);
}
if (Printer.debug) Printer.debug(" DirectClip.setFramePosition: "
