Re: RFH: ALSA output bundle for cynthiune.app

[Yavor Doganov]

В 18:00 +0200 на 11.02.2010 (чт), Yavor Doganov написа:
> В 18:19 +0200 на 09.02.2010 (вт), Yavor Doganov написа:
> > I wrote one fairly quickly, but I experience severe memory 
> > clobbering problems, especially at work, where my workstation is a
> > dual-core Pentium IV. 
> 
> It looks like there's nothing wrong with my code; it's a bug in
> GNUstep Base (most probably NSAutoreleasePool/NSRunLoop/NSThread).
> I'm trying to track it down so that we can apply the fix in Debian.
> Everything is working properly for me with GNUstep trunk.
> 
> You can reproduce the *same* bug fairly easily even without my patch
> -- choose a song from the playlist, and then press the blue "i" button
> on the toolbar (or select Tools->Song Inspector... from the menu),
> then press the "MB" button in the inspector window.  This spawns
> another thread to query the MusicBrainz server (regardless of the
> "threadness", of the chosen output bundle, as it has nothing to do
> with it) which always results in a crash.

So, the bug is unfortunately still there, but is only reproducible when
the package is built with no optimization.  Weird...

Since the default is -O2, I think it is OK to upload it now.  Here's an
updated patch -- the new thing is migrating the settings for existing
users.  One annoyance is that -[GeneralPreference _initDefaults] is
called at least three times during initialization, and because of the
modal dialog the user has to press the button thrice :-(  I couldn't
find a way to circumvent this.

I'd appreciate a review and eventually testing; thanks.

diff -u cynthiune.app-0.9.5/debian/copyright 
cynthiune.app-0.9.5/debian/copyright
--- cynthiune.app-0.9.5/debian/copyright
+++ cynthiune.app-0.9.5/debian/copyright
@@ -10,6 +10,7 @@
 Copyright:
 
     Copyright (C) 2002-2006 Wolfgang Sourdeau <address@hidden>
+    Copyright (C) 2010 Free Software Foundation, Inc.
 
     Cynthiune is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
diff -u cynthiune.app-0.9.5/debian/changelog 
cynthiune.app-0.9.5/debian/changelog
--- cynthiune.app-0.9.5/debian/changelog
+++ cynthiune.app-0.9.5/debian/changelog
@@ -1,3 +1,16 @@
+cynthiune.app (0.9.5-11) unstable; urgency=low
+
+  * debian/patches/ALSA.patch: New; implement an ALSA output bundle and
+    make it the default on GNU/Linux (Closes: #576228).
+  * debian/patches/kfreebsd-OSS-default.patch: Remove, default now handled
+    with the above patch.
+  * debian/patches/series: Update.
+  * debian/control (Build-Depends): Add libasound2-dev [linux-any].
+    (Standards-Version): Set to 3.9.1 (no changes needed).
+  * debian/copyright: Update copyright holders/years.
+
+ -- Yavor Doganov <address@hidden>  Fri, 03 Sep 2010 10:35:09 +0300
+
 cynthiune.app (0.9.5-10) unstable; urgency=low
 
   * debian/patches/compilation-errors.patch: Fix FTBFS with
diff -u cynthiune.app-0.9.5/debian/control cynthiune.app-0.9.5/debian/control
--- cynthiune.app-0.9.5/debian/control
+++ cynthiune.app-0.9.5/debian/control
@@ -17,10 +17,11 @@
               libmusicbrainz4-dev,
               libmpcdec-dev (>= 2:0.1~r435),
               libartsc0-dev,
+              libasound2-dev [linux-any],
               quilt,
               imagemagick
 Vcs-Arch: 
http://arch.debian.org/arch/pkg-gnustep/gnustep/cynthiune-app--debian--1.0
-Standards-Version: 3.8.4
+Standards-Version: 3.9.1
 Homepage: http://organact.mine.nu/~wolfgang/cynthiune/
 
 Package: cynthiune.app
diff -u cynthiune.app-0.9.5/debian/patches/series 
cynthiune.app-0.9.5/debian/patches/series
--- cynthiune.app-0.9.5/debian/patches/series
+++ cynthiune.app-0.9.5/debian/patches/series
@@ -1,4 +1,4 @@
-kfreebsd-OSS-default.patch
+ALSA.patch
 WindowsMedia.patch
 Bundles-install-dir.patch
 link-libs.patch
reverted:
--- cynthiune.app-0.9.5/debian/patches/kfreebsd-OSS-default.patch
+++ cynthiune.app-0.9.5.orig/debian/patches/kfreebsd-OSS-default.patch
@@ -1,18 +0,0 @@
-2009-04-09  Yavor Doganov  <address@hidden>
-
-       * GeneralPreference.m: Let -[GeneralPreference _initDefaults]
-          create the default output bundle to be OSS on GNU/kFreeBSD
-          too; this fixes some unpleasant glitches with the fallback
-          Esound bundle.
-
---- cynthiune.app-0.9.5.orig/GeneralPreference.m
-+++ cynthiune.app-0.9.5/GeneralPreference.m
-@@ -44,7 +44,7 @@
- #ifdef __MACOSX__
- #define defaultOutputBundle @"MacOSXPlayer"
- #else
--#ifdef __linux__
-+#if defined (__linux__) || defined (__GLIBC__)
- #define defaultOutputBundle @"OSS"
- #else
- #ifdef __WIN32__
only in patch2:
unchanged:
--- cynthiune.app-0.9.5.orig/debian/patches/ALSA.patch
+++ cynthiune.app-0.9.5/debian/patches/ALSA.patch
@@ -0,0 +1,402 @@
+2010-09-03  Yavor Doganov  <address@hidden>
+
+       Implement a threaded ALSA output bundle.
+       * Bundles/ALSA/ALSA.h:
+       * Bundles/ALSA/ALSA.m:
+       * Bundles/ALSA/GNUmakefile:
+       * Bundles/ALSA/GNUmakefile.preamble: New file.
+       * GNUmakefile (BUNDLES): Conditionally add ALSA.
+       * GeneralPreference.m: Make ALSA the default on GNU/Linux.
+       (-[GeneralPreference _initDefaults]): Add migration code for
+       existing users.
+
+--- cynthiune.app-0.9.5.orig/GNUmakefile
++++ cynthiune.app-0.9.5/GNUmakefile
+@@ -94,6 +94,14 @@
+ 
+ else
+ 
++ifeq (linux-gnu,$(GNUSTEP_TARGET_OS))
++
++ifneq (yes,$(disable-alsa))
++BUNDLES += ALSA
++endif
++
++endif
++
+ ifneq (yes,$(disable-oss))
+ BUNDLES += OSS
+ endif
+--- /dev/null
++++ cynthiune.app-0.9.5/Bundles/ALSA/ALSA.h
+@@ -0,0 +1,40 @@
++/* ALSA.h - this file is part of Cynthiune                       -*-objc-*-
++ *
++ * Copyright (C) 2010 Free Software Foundation, Inc.
++ *
++ * Author: Yavor Doganov <address@hidden>
++ *
++ * Cynthiune is free software: you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; either version 2, or (at your option)
++ * any later version.
++ *
++ * Cynthiune is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program; see the file COPYING.  If not, write to
++ * the Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
++ * Boston, MA 02110-1301, USA.
++ */
++
++#ifndef ALSA_H
++#define ALSA_H
++
address@hidden ALSA : NSObject <CynthiuneBundle, Output>
++{
++  id parentPlayer;
++  snd_pcm_t *pcm_handle;
++
++  BOOL stopRequested;
++
++  unsigned int channels;
++  unsigned long rate;
++
++  unsigned char buffer[DEFAULT_BUFFER_SIZE];
++}
address@hidden
++
++#endif /* ALSA_H */
+--- /dev/null
++++ cynthiune.app-0.9.5/Bundles/ALSA/ALSA.m
+@@ -0,0 +1,169 @@
++/* ALSA.m - this file is part of Cynthiune
++ *
++ * Copyright (C) 2010 Free Software Foundation, Inc.
++ *
++ * Author: Yavor Doganov <address@hidden>
++ *
++ * Cynthiune is free software: you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; either version 2, or (at your option)
++ * any later version.
++ *
++ * Cynthiune is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program; see the file COPYING.  If not, write to
++ * the Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
++ * Boston, MA 02110-1301, USA.
++ */
++
++#ifndef _REENTRANT
++#define _REENTRANT 1
++#endif
++
++#import <AppKit/AppKit.h>
++
++#import <Cynthiune/CynthiuneBundle.h>
++#import <Cynthiune/Output.h>
++#import <Cynthiune/utils.h>
++
++#include <alsa/asoundlib.h>
++
++#import "ALSA.h"
++
++#define LOCALIZED(X) _b ([ALSA class], X)
++
++static char *device = "default";
++
address@hidden ALSA
+++ (NSString *) bundleDescription
++{
++  return LOCALIZED (@"Output plug-in for ALSA");
++}
++
+++ (NSArray *) bundleCopyrightStrings
++{
++  return [NSArray arrayWithObjects:
++                    LOCALIZED (@"Copyright (C) 2010 Free Software Foundation,"
++                             @" Inc."),
++                nil];
++}
++
+++ (BOOL) isThreaded
++{
++  return YES;
++}
++
++- (void) setParentPlayer: (id) aPlayer;
++{
++  parentPlayer = aPlayer;
++}
++
++- (id) init
++{
++  if ((self = [super init]))
++    {
++      parentPlayer = nil;
++      pcm_handle = NULL;
++      channels = 0;
++      rate = 0;
++      stopRequested = NO;
++    }
++
++  return self;
++}
++
++- (BOOL) openDevice
++{
++  int err;
++  BOOL result = NO;
++
++  if ((err = snd_pcm_open (&pcm_handle, device, SND_PCM_STREAM_PLAYBACK, 0))
++      < 0)
++    NSRunAlertPanel (LOCALIZED (@"Error"),
++                   LOCALIZED (@"Failed to open the ALSA device:\n%s"),
++                   LOCALIZED (@"OK"), NULL, NULL, snd_strerror (err));
++  else if ((err = snd_pcm_set_params (pcm_handle, SND_PCM_FORMAT_S16,
++                                    SND_PCM_ACCESS_RW_INTERLEAVED,
++                                    channels, rate, 1, 5000000)) < 0)
++    NSRunAlertPanel (LOCALIZED (@"Error"),
++                   LOCALIZED (@"Failed to set device parameters:\n%s"),
++                   LOCALIZED (@"OK"), NULL, NULL, snd_strerror (err));
++  else if ((err = snd_pcm_prepare (pcm_handle)) < 0)
++    NSRunAlertPanel (LOCALIZED (@"Error"),
++                   LOCALIZED (@"Failed to prepare the ALSA device for "
++                              @"playing:\n%s"),
++                   LOCALIZED (@"OK"), NULL, NULL, snd_strerror (err));
++  else
++    result = YES;
++
++  return result;
++}
++
++- (BOOL) prepareDeviceWithChannels: (unsigned int) numberOfChannels
++                           andRate: (unsigned long) sampleRate
++{
++  channels = numberOfChannels;
++  rate = sampleRate;
++
++  return YES;
++}
++
++- (void) closeDevice
++{
++  while (stopRequested)
++    [NSThread sleepUntilDate: [NSDate dateWithTimeIntervalSinceNow: 0.1]];
++  snd_pcm_close (pcm_handle);
++}
++
++- (void) threadLoop
++{
++  NSAutoreleasePool *pool = [NSAutoreleasePool new];
++
++  while (!stopRequested)
++    {
++      snd_pcm_sframes_t written;
++      snd_pcm_uframes_t frames;
++      int bufferSize;
++
++      bufferSize = [parentPlayer readNextChunk: buffer
++                                    withSize: DEFAULT_BUFFER_SIZE];
++
++      if (bufferSize > 0)
++      {
++        frames = snd_pcm_bytes_to_frames (pcm_handle, bufferSize);
++        written = snd_pcm_writei (pcm_handle, buffer, frames);
++
++        if (written < 0)
++          {
++            NSLog (@"Failed writing to the ALSA device:\n"
++                   @"%s, trying to recover.\n", snd_strerror (written));
++            snd_pcm_recover (pcm_handle, written, 0);
++          }
++      }
++
++      if ([pool autoreleaseCount] > 50)
++      [pool emptyPool];
++    }
++
++  stopRequested = NO;
++  [pool release];
++}
++
++- (BOOL) startThread
++{
++  [NSThread detachNewThreadSelector: @selector(threadLoop)
++            toTarget: self
++            withObject: nil];
++
++  return YES;
++}
++
++- (void) stopThread
++{
++  stopRequested = YES;
++}
address@hidden
+--- /dev/null
++++ cynthiune.app-0.9.5/Bundles/ALSA/GNUmakefile
+@@ -0,0 +1,44 @@
++# GNUmakefile - this file is part of Cynthiune
++#
++# Copyright (C) 2010 Free Software Foundation, Inc.
++#
++# Author: Yavor Doganov <address@hidden>
++#
++# Cynthiune is free software: you can redistribute it and/or modify
++# it under the terms of the GNU General Public License as published by
++# the Free Software Foundation; either version 2, or (at your option)
++# any later version.
++#
++# Cynthiune is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU General Public License
++# along with this program; see the file COPYING.  If not, write to
++# the Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
++# Boston, MA 02110-1301, USA.
++
++include $(GNUSTEP_MAKEFILES)/common.make
++
++PACKAGE_NAME = ALSA
++BUNDLE_NAME = ALSA
++BUNDLE_EXTENSION = .output
++BUNDLE_INSTALL_DIR = $(GNUSTEP_LIBRARY)/Cynthiune
++ALSA_PRINCIPAL_CLASS = ALSA
++
++ALSA_HEADERS = ALSA.h
++
++ALSA_OBJC_FILES = ALSA.m
++
++ADDITIONAL_INCLUDE_DIRS += -I../../Frameworks
++
++FRAMEWORKS_DIRS = ../../Frameworks/Cynthiune
++FRAMEWORKS = Cynthiune
++
++include ../../frameworks.make
++
++-include GNUmakefile.preamble
++-include GNUmakefile.local
++include $(GNUSTEP_MAKEFILES)/bundle.make
++-include GNUmakefile.postamble
+--- /dev/null
++++ cynthiune.app-0.9.5/Bundles/ALSA/GNUmakefile.preamble
+@@ -0,0 +1,31 @@
++# GNUmakefile.preamble - this file is part of Cynthiune          
-*-makefile-gmake-*-
++#
++# Copyright (C) 2010 Free Software Foundation, Inc.
++#
++# Author: Yavor Doganov <address@hidden>
++#
++# Cynthiune is free software: you can redistribute it and/or modify
++# it under the terms of the GNU General Public License as published by
++# the Free Software Foundation; either version 2, or (at your option)
++# any later version.
++#
++# Cynthiune is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU General Public License
++# along with this program; see the file COPYING.  If not, write to
++# the Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
++# Boston, MA 02110-1301, USA.
++
++ALSA_CFLAGS := $(shell pkg-config --cflags alsa)
++ALSA_LIBS := $(shell pkg-config --libs alsa)
++
++ADDITIONAL_INCLUDE_DIRS += $(ALSA_CFLAGS)
++BUNDLE_LIBS += $(ALSA_LIBS)
++ADDITIONAL_OBJCFLAGS += -Wall -Wno-import
++
++ifeq ($(debug), yes)
++ADDITIONAL_LDFLAGS += -lmcheck
++endif
+--- cynthiune.app-0.9.5.orig/GeneralPreference.m
++++ cynthiune.app-0.9.5/GeneralPreference.m
+@@ -20,11 +20,7 @@
+  * Boston, MA 02111-1307, USA.
+  */
+ 
+-#import <AppKit/NSBox.h>
+-#import <AppKit/NSWindow.h>
+-#import <AppKit/NSNibLoading.h>
+-#import <AppKit/NSPopUpButton.h>
+-#import <AppKit/NSTextField.h>
++#import <AppKit/AppKit.h>
+ 
+ #import <Foundation/NSArray.h>
+ #import <Foundation/NSDictionary.h>
+@@ -45,12 +41,12 @@
+ #define defaultOutputBundle @"MacOSXPlayer"
+ #else
+ #ifdef __linux__
+-#define defaultOutputBundle @"OSS"
++#define defaultOutputBundle @"ALSA"
+ #else
+ #ifdef __WIN32__
+ #define defaultOutputBundle @"WaveOut"
+ #else
+-#define defaultOutputBundle @"Esound"
++#define defaultOutputBundle @"OSS"
+ #endif
+ #endif
+ #endif
+@@ -90,6 +86,46 @@
+           [preference setObject: outputBundle forKey: @"OutputBundle"];
+         }
+ 
++#ifdef __linux__
++      if (![outputBundle isEqualToString: @"ALSA"])
++      {
++        NSString *migrated;
++        int choice;
++
++        /* Check if the user has already made her choice about which
++        output bundle to use.  The default `SuggestedALSA' is to
++        ensure that the dialog does not appear every time on
++        startup, even if the user's preference is OSS or something
++        else.  */
++        migrated = [preference objectForKey: @"SuggestedALSA"];
++
++        if (!migrated)
++          {
++            choice =
++              NSRunAlertPanel (_(@"Deprecated output bundle detected"),
++                               _(@"ALSA is the recommended output bundle\n"
++                                 @"on GNU/Linux systems, but %@ is "
++                                 @"currently used.\nWould you like to "
++                                 @"switch to ALSA now?"),
++                               _(@"Yes"), _(@"No"),
++                               _(@"No, don't ask anymore"),
++                               outputBundle);
++
++            switch (choice)
++              {
++              case NSAlertDefaultReturn:
++                [preference setObject: @"ALSA" forKey: @"OutputBundle"];
++                [preference setObject: @"YES" forKey: @"SuggestedALSA"];
++                break;
++              case NSAlertAlternateReturn:
++                break;
++              case NSAlertOtherReturn:
++                [preference setObject: @"YES" forKey: @"SuggestedALSA"];
++              }
++          }
++      }
++#endif
++
+       playlistFormat = [preference objectForKey: @"PlaylistFormat"];
+       if (!playlistFormat
+           || !([playlistFormat isEqualToString: @"m3u"]