Updated patch for a Windows implementation of play-sound - 2

Updated patch for a Windows implementation of play-sound - 2

[Ben Key]

[-- Attachment #1: Type: text/plain, Size: 2039 bytes --]

This is another updated version of my patch to provide a Windows compatible
implementation of play-sound.  In this update I incorporated several more
suggestions made to me by members of this list.  The largest change between
this patch and my previous one is thaI am no longer adding a file
w32sound.c.  Instead I am using sound.c.

The changes made by this patch are as follows:

* nt/nmake.defs and nt/gmake.defs have been modified so that the constant
WINMM is defined in such a way to indicate how to link with the library
WINMM.LIB.  This constant was defined much like the constants ADVAPI32,
COMDLG32, GDI32, MPR, SHELL32, USER32, and WSOCK32.

* src/makefile.w32-in has been modified to compile sound.c and to link with
WINMM.LIB.

* src/s/ms-w32.h has been modified so that the preprocessor symbol
HAVE_SOUND is defined.  This enables sound support in Emacs.

* The code that was previously in w32sound.c has been merged into sound.c.
sound.c has been reorganized sightly to make it easier for the platform
specific portions of the code to be enclosed by #ifdef statements and has
been divided up into well commented sections to ensure that there is no
misunderstanding about the purpose of a particular #ifdef block.  In order
to do this in as clear and orderly a fashion as possible, I had to move some
blocks of code up or down to deal with situations where code that works in
all platforms was thrown right in the middle of a large block of platform
specific code.

So far I have tested this patch under Windows using both MSVC 6.0 and MinGW
2.0.  I have not been able to test this patch under Linux because I have not
yet finished setting up my Linux machine (maybe tomorrow).  I cannot test
this patch under any other platforms besides Windows and after tomorrow,
Linux, because I do not have access to any other platforms.

I would appreciate it if someone would apply this patch on a platform that I
do not have access to yet has sound support and see if sound.c will compile
properly on that platform (perhaps Mac OSX).

[-- Attachment #2: emacs-21.3.50-w32-play-sound.diff --]
[-- Type: application/octet-stream, Size: 19585 bytes --]

--- _21.3/nt/nmake.defs	2002-01-04 09:04:53.000000000 -0500
+++ 21.3/nt/nmake.defs	2002-10-23 22:27:03.000000000 -0400
@@ -123,6 +123,7 @@
 SHELL32		= shell32.lib
 USER32		= user32.lib
 WSOCK32		= wsock32.lib
+WINMM     = winmm.lib
 
 !ifdef NOOPT
 DEBUG_CFLAGS	= -DEMACSDEBUG
--- _21.3/nt/gmake.defs	2002-01-04 09:04:26.000000000 -0500
+++ 21.3/nt/gmake.defs	2002-10-23 22:27:03.000000000 -0400
@@ -166,6 +166,7 @@
 SHELL32		= -lshell32
 USER32		= -luser32
 WSOCK32		= -lwsock32
+WINMM     = -lwinmm
 
 ifdef NOOPT
 DEBUG_CFLAGS	= -DEMACSDEBUG
--- _21.3/src/makefile.w32-in	2002-09-04 02:20:50.000000000 -0400
+++ 21.3/src/makefile.w32-in	2002-10-23 23:11:10.000000000 -0400
@@ -94,6 +94,7 @@
 	$(BLD)/regex.$(O)			\
 	$(BLD)/scroll.$(O)			\
 	$(BLD)/search.$(O)			\
+	$(BLD)/sound.$(O)			\
 	$(BLD)/syntax.$(O)			\
 	$(BLD)/sysdep.$(O)			\
 	$(BLD)/term.$(O)			\
@@ -133,6 +134,7 @@
 	$(TLIB1)	\
 	$(TLIBW32)	\
 	$(TLASTLIB)	\
+	$(WINMM)    \
 	$(ADVAPI32)	\
 	$(GDI32)	\
 	$(COMDLG32)	\
@@ -1094,6 +1096,13 @@
 	$(SRC)/w32bdf.h \
 	$(SRC)/w32gui.h
 
+$(BLD)/sound.$(O) : \
+	$(SRC)/sound.c \
+	$(SRC)/lisp.h \
+	$(SRC)/dispextern.h \
+	$(SRC)/atimer.h \
+	$(SRC)/syssignal.h
+
 $(BLD)/strftime.$(O) : \
 	$(SRC)/strftime.c \
 	$(EMACS_ROOT)/src/s/ms-w32.h \
--- _21.3/src/s/ms-w32.h	2002-05-03 16:42:33.000000000 -0400
+++ 21.3/src/s/ms-w32.h	2002-10-23 22:27:03.000000000 -0400
@@ -217,6 +217,7 @@
 #define MAXPATHLEN      _MAX_PATH
 #endif
 
+#define HAVE_SOUND  1
 #define LISP_FLOAT_TYPE 1
 
 #undef  HAVE_SYS_SELECT_H
--- _21.3/src/sound.c	2002-07-19 10:27:20.000000000 -0400
+++ 21.3/src/sound.c	2002-10-24 22:08:38.000000000 -0400
@@ -21,10 +21,30 @@
 /* Written by Gerd Moellmann <gerd@gnu.org>.  Tested with Luigi's
    driver on FreeBSD 2.2.7 with a SoundBlaster 16.  */
 
+/*
+  Modified by Ben Key <Bkey1@tampabay.rr.com> to add a partial
+  implementation of the play-sound specification for Windows.
+
+  Notes:
+  In the Windows implementation of play-sound-internal only the
+  :file and :volume keywords are supported.  The :device keyword,
+  if present, is ignored.  The :data keyword, if present, will
+  cause an error to be generated.
+
+  The Windows implementation of play-sound is implemented via the
+  Win32 API functions mciSendString, waveOutGetVolume, and
+  waveOutGetVolume which are exported by Winmm.dll.
+*/
+
 #include <config.h>
 
 #if defined HAVE_SOUND
 
+/*
+  @@>>
+  BEGIN SECTION: Common Includes
+  Include files that are common to all platforms
+*/
 #include <fcntl.h>
 #include <unistd.h>
 #include <sys/types.h>
@@ -34,6 +54,18 @@
 #include "atimer.h"
 #include <signal.h>
 #include "syssignal.h"
+/*
+  END SECTION: Common Includes
+  Include files that are common to all platforms
+  <<@@
+*/
+
+/*
+  @@>>
+  BEGIN SECTION: Non Windows Includes
+  Include files that are not used in Microsoft Windows
+*/
+#if !defined(WINDOWSNT)
 
 #ifndef MSDOS
 #include <sys/ioctl.h>
@@ -51,12 +83,86 @@
 #ifdef HAVE_SOUNDCARD_H
 #include <soundcard.h>
 #endif
+/*
+  END SECTION: Non Windows Includes
+  Include files that are not used in Microsoft Windows
+  <<@@
+*/
+
+#else /* #if !defined(WINDOWSNT) */
+
+/*
+  @@>>
+  BEGIN SECTION: Windows Specific Includes
+  Include files that are only in Microsoft Windows
+*/
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <limits.h>
+#include <windows.h>
+#include <mmsystem.h>
+/*
+  END SECTION: Windows Specific Includes
+  Include files that are only in Microsoft Windows
+  <<@@
+*/
+
+#endif /* #if !defined(WINDOWSNT) */
+
+/*
+  @@>>
+  BEGIN SECTION: Common Definitions
+  Contains macro, type, and variable definitions that are
+  common to all platforms.  Also contains function prototypes
+  for functions that are common to all platforms.
+*/
+#define abs(X)    ((X) < 0 ? -(X) : (X))
+
+/* Symbols.  */
+
+extern Lisp_Object QCfile, QCdata;
+Lisp_Object QCvolume, QCdevice;
+Lisp_Object Qsound;
+Lisp_Object Qplay_sound_functions;
+
+/* Indices of attributes in a sound attributes vector.  */
+
+enum sound_attr
+{
+  SOUND_FILE,
+  SOUND_DATA,
+  SOUND_DEVICE,
+  SOUND_VOLUME,
+  SOUND_ATTR_SENTINEL
+};
+
+static void sound_perror P_ ((char *));
+static void sound_warning P_ ((char *));
+static int parse_sound P_ ((Lisp_Object, Lisp_Object *));
+
+/*
+  END SECTION: Common Definitions
+  Contains macro, type, and variable definitions that are
+  common to all platforms.  Also contains function prototypes
+  for functions that are common to all platforms.
+  <<@@
+*/
+
+/*
+  @@>>
+  BEGIN SECTION: Non Windows Definitions
+  Contains macro, type, and variable definitions that are
+  only used on platforms other then Microsoft Windows.
+  Also contains function prototypes for functions that
+  are only used on platforms other then Microsoft Windows.
+*/
+#if !defined(WINDOWSNT)
 
 #ifndef DEFAULT_SOUND_DEVICE
 #define DEFAULT_SOUND_DEVICE "/dev/dsp"
 #endif
 
-#define abs(X)    ((X) < 0 ? -(X) : (X))
 
 /* Structure forward declarations.  */
 
@@ -203,24 +309,6 @@
   void (* play) P_ ((struct sound *s, struct sound_device *sd));
 };
 
-/* Indices of attributes in a sound attributes vector.  */
-
-enum sound_attr
-{
-  SOUND_FILE,
-  SOUND_DATA,
-  SOUND_DEVICE,
-  SOUND_VOLUME,
-  SOUND_ATTR_SENTINEL
-};
-
-/* Symbols.  */
-
-extern Lisp_Object QCfile, QCdata;
-Lisp_Object QCvolume, QCdevice;
-Lisp_Object Qsound;
-Lisp_Object Qplay_sound_functions;
-
 /* These are set during `play-sound-internal' so that sound_cleanup has
    access to them.  */
 
@@ -235,9 +323,6 @@
 static void vox_choose_format P_ ((struct sound_device *, struct sound *));
 static void vox_init P_ ((struct sound_device *));
 static void vox_write P_ ((struct sound_device *, const char *, int));
-static void sound_perror P_ ((char *));
-static void sound_warning P_ ((char *));
-static int parse_sound P_ ((Lisp_Object, Lisp_Object *));
 static void find_sound_type P_ ((struct sound *));
 static u_int32_t le2hl P_ ((u_int32_t));
 static u_int16_t le2hs P_ ((u_int16_t));
@@ -251,12 +336,46 @@
 static u_int16_t be2hs P_ ((u_int16_t));
 #endif
 
+/*
+  END SECTION: Non Windows Definitions
+  Contains macro, type, and variable definitions that are
+  only used on platforms other then Microsoft Windows.
+  Also contains function prototypes for functions that
+  are only used on platforms other then Microsoft Windows.
+  <<@@
+*/
+#else /* #if !defined(WINDOWSNT) */
+
+/*
+  @@>>
+  BEGIN SECTION: Windows Specific Definitions
+  Contains macro, type, and variable definitions that are
+  specific to Microsoft Windows.
+  Also contains function prototypes for functions that
+  are specific to Microsoft Windows.
+*/
+static int do_play_sound P_ ((const char *,unsigned int));
+/*
+  END SECTION: Windows Specific Definitions
+  Contains macro, type, and variable definitions that are
+  specific to Microsoft Windows.
+  Also contains function prototypes for functions that
+  are specific to Microsoft Windows.
+  <<@@
+*/
+#endif /* #if !defined(WINDOWSNT) */
 
 \f
 /***********************************************************************
 			       General
  ***********************************************************************/
 
+/*
+  @@>>
+  BEGIN SECTION: Common functions
+  Functions that are common to all platformas
+*/
+
 /* Like perror, but signals an error.  */
 
 static void
@@ -327,10 +446,21 @@
   attrs[SOUND_DEVICE] = Fplist_get (sound, QCdevice);
   attrs[SOUND_VOLUME] = Fplist_get (sound, QCvolume);
 
+#if !defined(WINDOWSNT)
   /* File name or data must be specified.  */
   if (!STRINGP (attrs[SOUND_FILE])
       && !STRINGP (attrs[SOUND_DATA]))
     return 0;
+#else /* #if !defined(WINDOWSNT) */
+  /*
+    Data is not supported in Windows.  Therefore a
+    File name MUST be supplied.
+  */
+  if (!STRINGP(attrs[SOUND_FILE]))
+    {
+      return 0;
+    }
+#endif /* #if !defined(WINDOWSNT) */
 
   /* Volume must be in the range 0..100 or unspecified.  */
   if (!NILP (attrs[SOUND_VOLUME]))
@@ -351,14 +481,31 @@
 	return 0;
     }
 
+#if !defined(WINDOWSNT)
   /* Device must be a string or unspecified.  */
   if (!NILP (attrs[SOUND_DEVICE])
       && !STRINGP (attrs[SOUND_DEVICE]))
     return 0;
-
+#endif  /* #if !defined(WINDOWSNT) */
+  /*
+    Since device is ignored in Windows, it does not matter
+    what it is.
+   */
   return 1;
 }
 
+/*
+  END SECTION: Common functions
+  Functions that are common to all platformas
+  <<@@
+*/
+
+/*
+  @@>>
+  BEGIN SECTION: Non Windows functions
+  Functions that are only used on non Windows platforms
+*/
+#if !defined(WINDOWSNT)
 
 /* Find out the type of the sound file whose file descriptor is FD.
    S is the sound file structure to fill in.  */
@@ -389,105 +536,6 @@
   return Qnil;
 }
 
-
-DEFUN ("play-sound-internal", Fplay_sound_internal, Splay_sound_internal, 1, 1, 0,
-       doc: /* Play sound SOUND.
-
-Internal use only, use `play-sound' instead.  */)
-     (sound)
-     Lisp_Object sound;
-{
-  Lisp_Object attrs[SOUND_ATTR_SENTINEL];
-  Lisp_Object file;
-  struct gcpro gcpro1, gcpro2;
-  struct sound_device sd;
-  struct sound s;
-  Lisp_Object args[2];
-  int count = SPECPDL_INDEX ();
-
-  file = Qnil;
-  GCPRO2 (sound, file);
-  bzero (&sd, sizeof sd);
-  bzero (&s, sizeof s);
-  current_sound_device = &sd;
-  current_sound = &s;
-  record_unwind_protect (sound_cleanup, Qnil);
-  s.header = (char *) alloca (MAX_SOUND_HEADER_BYTES);
-
-  /* Parse the sound specification.  Give up if it is invalid.  */
-  if (!parse_sound (sound, attrs))
-    error ("Invalid sound specification");
-
-  if (STRINGP (attrs[SOUND_FILE]))
-    {
-      /* Open the sound file.  */
-      s.fd = openp (Fcons (Vdata_directory, Qnil),
-		    attrs[SOUND_FILE], Qnil, &file, Qnil);
-      if (s.fd < 0)
-	sound_perror ("Could not open sound file");
-
-      /* Read the first bytes from the file.  */
-      s.header_size = emacs_read (s.fd, s.header, MAX_SOUND_HEADER_BYTES);
-      if (s.header_size < 0)
-	sound_perror ("Invalid sound file header");
-    }
-  else
-    {
-      s.data = attrs[SOUND_DATA];
-      s.header_size = min (MAX_SOUND_HEADER_BYTES, SBYTES (s.data));
-      bcopy (SDATA (s.data), s.header, s.header_size);
-    }
-
-  /* Find out the type of sound.  Give up if we can't tell.  */
-  find_sound_type (&s);
-
-  /* Set up a device.  */
-  if (STRINGP (attrs[SOUND_DEVICE]))
-    {
-      int len = SCHARS (attrs[SOUND_DEVICE]);
-      sd.file = (char *) alloca (len + 1);
-      strcpy (sd.file, SDATA (attrs[SOUND_DEVICE]));
-    }
-
-  if (INTEGERP (attrs[SOUND_VOLUME]))
-    sd.volume = XFASTINT (attrs[SOUND_VOLUME]);
-  else if (FLOATP (attrs[SOUND_VOLUME]))
-    sd.volume = XFLOAT_DATA (attrs[SOUND_VOLUME]) * 100;
-
-  args[0] = Qplay_sound_functions;
-  args[1] = sound;
-  Frun_hook_with_args (2, args);
-
-  /* There is only one type of device we currently support, the VOX
-     sound driver.  Set up the device interface functions for that
-     device.  */
-  vox_init (&sd);
-
-  /* Open the device.  */
-  sd.open (&sd);
-
-  /* Play the sound.  */
-  s.play (&s, &sd);
-
-  /* Close the input file, if any.  */
-  if (!STRINGP (s.data))
-    {
-      emacs_close (s.fd);
-      s.fd = -1;
-    }
-
-  /* Close the device.  */
-  sd.close (&sd);
-
-  /* Clean up.  */
-  current_sound_device = NULL;
-  current_sound = NULL;
-  UNGCPRO;
-  unbind_to (count, Qnil);
-  return Qnil;
-}
-
-\f
 /***********************************************************************
 			Byte-order Conversion
  ***********************************************************************/
@@ -909,7 +957,6 @@
   sd->write = vox_write;
 }
 
-
 /* Write NBYTES bytes from BUFFER to device SD.  */
 
 static void
@@ -923,7 +970,241 @@
     sound_perror ("Error writing to sound device");
 }
 
+/*
+  END SECTION: Non Windows functions
+  Functions that are only used on non Windows platforms
+  <<@@
+*/
+#else /* #if !defined(WINDOWSNT) */
+
+/*
+  @@>>
+  BEGIN SECTION: Windows specific functions
+  Functions that are only used Microsoft Windows
+*/
+
+static int
+do_play_sound(psz_file,ui_volume)
+    const char * psz_file;
+    unsigned int ui_volume;
+{
+  int i_result=0;
+  MCIERROR mci_error=0;
+  char sz_cmd_buf[520]={0};
+  char sz_ret_buf[520]={0};
+  MMRESULT mm_result=MMSYSERR_NOERROR;
+  unsigned int ui_volume_org=0;
+  BOOL b_reset_volume=FALSE;
+  memset(sz_cmd_buf,0,sizeof(sz_cmd_buf));
+  memset(sz_ret_buf,0,sizeof(sz_ret_buf));
+  sprintf(
+    sz_cmd_buf,
+    "open \"%s\" alias GNUEmacs_PlaySound_Device wait",
+    psz_file);
+  mci_error=mciSendString(sz_cmd_buf,sz_ret_buf,520,NULL);
+  if (mci_error!=0)
+    {
+      sound_warning(
+        "The open mciSendString command failed to open\n"
+        "the specified sound file");
+      i_result=(int)mci_error;
+      return i_result;
+    }
+  if (ui_volume>0 && ui_volume!=UINT_MAX)
+    {
+      mm_result=waveOutGetVolume((HWAVEOUT)WAVE_MAPPER,&ui_volume_org);
+      if (mm_result==MMSYSERR_NOERROR)
+        {
+          b_reset_volume=TRUE;
+          mm_result=waveOutSetVolume((HWAVEOUT)WAVE_MAPPER,ui_volume);
+          if (mm_result!=MMSYSERR_NOERROR)
+            {
+              sound_warning(
+                "waveOutSetVolume failed to set the volume level\n"
+                "of the WAVE_MAPPER device.\n"
+                "As a result, the user selected volume level will\n"
+                "not be used.");
+            }
+        }
+      else
+        {
+          sound_warning(
+            "waveOutGetVolume failed to obtain the original\n"
+            "volume level of the WAVE_MAPPER device.\n"
+            "As a result, the user selected volume level will\n"
+            "not be used.");
+        }
+    }
+  memset(sz_cmd_buf,0,sizeof(sz_cmd_buf));
+  memset(sz_ret_buf,0,sizeof(sz_ret_buf));
+  strcpy(sz_cmd_buf,"play GNUEmacs_PlaySound_Device wait");
+  mci_error=mciSendString(sz_cmd_buf,sz_ret_buf,520,NULL);
+  if (mci_error!=0)
+    {
+      sound_warning(
+        "The play mciSendString command failed to play the\n"
+        "opened sound file.");
+      i_result=(int)mci_error;
+    }
+  memset(sz_cmd_buf,0,sizeof(sz_cmd_buf));
+  memset(sz_ret_buf,0,sizeof(sz_ret_buf));
+  strcpy(sz_cmd_buf,"close GNUEmacs_PlaySound_Device wait");
+  mci_error=mciSendString(sz_cmd_buf,sz_ret_buf,520,NULL);
+  if (b_reset_volume==TRUE)
+    {
+      mm_result=waveOutSetVolume((HWAVEOUT)WAVE_MAPPER,ui_volume_org);
+      if (mm_result!=MMSYSERR_NOERROR)
+        {
+          sound_warning(
+            "waveOutSetVolume failed to reset the original volume\n"
+            "level of the WAVE_MAPPER device.");
+        }
+    }
+  return i_result;
+}
+
+/*
+  END SECTION: Windows specific functions
+  Functions that are only used Microsoft Windows
+  <<@@
+*/
+
+#endif /* #if !defined(WINDOWSNT) */
+
+
+DEFUN ("play-sound-internal", Fplay_sound_internal, Splay_sound_internal, 1, 1, 0,
+       doc: /* Play sound SOUND.
+
+Internal use only, use `play-sound' instead.\n  */)
+     (sound)
+     Lisp_Object sound;
+{
+  Lisp_Object attrs[SOUND_ATTR_SENTINEL];
+  int count = SPECPDL_INDEX ();
+
+#if !defined(WINDOWSNT)
+  Lisp_Object file;
+  struct gcpro gcpro1, gcpro2;
+  struct sound_device sd;
+  struct sound s;
+  Lisp_Object args[2];
+#else /* #if !defined(WINDOWSNT) */
+  int len=0;
+  char * psz_file=NULL;
+  unsigned int ui_volume_tmp=UINT_MAX;
+  unsigned int ui_volume=UINT_MAX;
+  int i_result=0;
+#endif /* #if !defined(WINDOWSNT) */
+
+  /* Parse the sound specification.  Give up if it is invalid.  */
+  if (!parse_sound (sound, attrs))
+    error ("Invalid sound specification");
+
+#if !defined(WINDOWSNT)
+  file = Qnil;
+  GCPRO2 (sound, file);
+  bzero (&sd, sizeof sd);
+  bzero (&s, sizeof s);
+  current_sound_device = &sd;
+  current_sound = &s;
+  record_unwind_protect (sound_cleanup, Qnil);
+  s.header = (char *) alloca (MAX_SOUND_HEADER_BYTES);
+
+  if (STRINGP (attrs[SOUND_FILE]))
+    {
+      /* Open the sound file.  */
+      s.fd = openp (Fcons (Vdata_directory, Qnil),
+		    attrs[SOUND_FILE], Qnil, &file, Qnil);
+      if (s.fd < 0)
+	sound_perror ("Could not open sound file");
+
+      /* Read the first bytes from the file.  */
+      s.header_size = emacs_read (s.fd, s.header, MAX_SOUND_HEADER_BYTES);
+      if (s.header_size < 0)
+	sound_perror ("Invalid sound file header");
+    }
+  else
+    {
+      s.data = attrs[SOUND_DATA];
+      s.header_size = min (MAX_SOUND_HEADER_BYTES, SBYTES (s.data));
+      bcopy (SDATA (s.data), s.header, s.header_size);
+    }
+
+  /* Find out the type of sound.  Give up if we can't tell.  */
+  find_sound_type (&s);
 
+  /* Set up a device.  */
+  if (STRINGP (attrs[SOUND_DEVICE]))
+    {
+      int len = SCHARS (attrs[SOUND_DEVICE]);
+      sd.file = (char *) alloca (len + 1);
+      strcpy (sd.file, SDATA (attrs[SOUND_DEVICE]));
+    }
+
+  if (INTEGERP (attrs[SOUND_VOLUME]))
+    sd.volume = XFASTINT (attrs[SOUND_VOLUME]);
+  else if (FLOATP (attrs[SOUND_VOLUME]))
+    sd.volume = XFLOAT_DATA (attrs[SOUND_VOLUME]) * 100;
+
+  args[0] = Qplay_sound_functions;
+  args[1] = sound;
+  Frun_hook_with_args (2, args);
+
+  /* There is only one type of device we currently support, the VOX
+     sound driver.  Set up the device interface functions for that
+     device.  */
+  vox_init (&sd);
+
+  /* Open the device.  */
+  sd.open (&sd);
+
+  /* Play the sound.  */
+  s.play (&s, &sd);
+
+  /* Close the input file, if any.  */
+  if (!STRINGP (s.data))
+    {
+      emacs_close (s.fd);
+      s.fd = -1;
+    }
+
+  /* Close the device.  */
+  sd.close (&sd);
+
+  /* Clean up.  */
+  current_sound_device = NULL;
+  current_sound = NULL;
+  UNGCPRO;
+#else /* #if !defined(WINDOWSNT) */
+  len=XSTRING(attrs[SOUND_FILE])->size;
+  psz_file=(char *)alloca(len+1);
+  strcpy(psz_file,XSTRING(attrs[SOUND_FILE])->data);
+  if (INTEGERP (attrs[SOUND_VOLUME]))
+    {
+      ui_volume_tmp=XFASTINT(attrs[SOUND_VOLUME]);
+    }
+  else if (FLOATP(attrs[SOUND_VOLUME]))
+    {
+      ui_volume_tmp=XFLOAT_DATA(attrs[SOUND_VOLUME])*100;
+    }
+  /*
+    Based on some experiments I have conducted, a value of 100 or less
+    for the sound volume is much too low.  You cannot even hear it.
+    A value of UINT_MAX indicates that you wish for the sound to played
+    at the maximum possible volume.  A value of UINT_MAX/2 plays the
+    sound at 50% maximum volume.  Therefore the value passed to do_play_sound
+    (and thus to waveOutSetVolume must be some fraction of UINT_MAX.
+    The following code adjusts the user specified volume level appropriately.
+   */
+  if (ui_volume_tmp>0 && ui_volume_tmp<=100)
+    {
+      ui_volume=ui_volume_tmp*(UINT_MAX/100);
+    }
+  i_result=do_play_sound(psz_file,ui_volume);
+#endif /* #if !defined(WINDOWSNT) */
+  unbind_to (count, Qnil);
+  return Qnil;
+}
 \f
 /***********************************************************************
 			    Initialization

[-- Attachment #3: dopatch21_3.sh --]
[-- Type: application/octet-stream, Size: 86 bytes --]

#!/bin/sh
patch --unified --quiet --strip=1 --input=emacs-21.3.50-w32-play-sound.diff

Re: Updated patch for a Windows implementation of play-sound - 2

[Stefan Monnier]

> This is another updated version of my patch to provide a Windows compatible
> implementation of play-sound.  In this update I incorporated several more
> suggestions made to me by members of this list.  The largest change between
> this patch and my previous one is thaI am no longer adding a file
> w32sound.c.  Instead I am using sound.c.

Thank you.

> * The code that was previously in w32sound.c has been merged into sound.c.
> sound.c has been reorganized sightly to make it easier for the platform
> specific portions of the code to be enclosed by #ifdef statements and has
> been divided up into well commented sections to ensure that there is no
> misunderstanding about the purpose of a particular #ifdef block.  In order

I value my vertical screen real-estate too much, I guess, but I find those
comments a bit too heavy.  A simple

	/* Common includes.  */

would be enough.  I like the code-reorg on OTOH.
Oh and the "normal" #ifdef style used in Emacs is:

	#if FOO
	#else /* !FOO */
	#endif /* !FOO */
or
	#if FOO
	#endif /* FOO */

> I would appreciate it if someone would apply this patch on a platform that I
> do not have access to yet has sound support and see if sound.c will compile
> properly on that platform (perhaps Mac OSX).

Compiled here on Linux.


	Stefan

RE: Updated patch for a Windows implementation of play-sound - 2

[Ben Key]

>  Oh and the "normal" #ifdef style used in Emacs is:
>
>	#if FOO
>	#else /* !FOO */
>	#endif /* !FOO */
> or
>	#if FOO
>	#endif /* FOO */

I find this style of #ifdef blocks to be much too cryptic for my tastes.
What does /* !FOO */ and /* FOO */ mean anyway.  I find that it is much
clearer if your comments for the #else and #endif portions state exactly
what #ifdef line they go to rather than some cryptic reference to what they
go to.  This style causes much less confusion and is a great help when you
are not really reading the code in detail but are just skimming through
looking for the place where you need to make a change.

This is one of the few places where my coding style differs from the GNU
coding standards.

I would like to thank you for testing this patch on Linux.  I have not yet
had a chance to do so myself.  I still do not have my Linux machine up and
running completely.  I am now trying to get Samba to work on it properly.

Re: Updated patch for a Windows implementation of play-sound - 2

[Kai Großjohann]

"Ben Key" <Bkey1@tampabay.rr.com> writes:

> This is one of the few places where my coding style differs from the GNU
> coding standards.

I think it's better to comply with the Emacs coding standards, for
subsequent editors probably won't update your special comments.  (I'm
especially referring to the @<< markers that are probably used by
folding mode or outline mode or somesuch.)

kai (having trouble with the space-before-open-paren policy himself :-)
-- 
~/.signature is: umop ap!sdn    (Frank Nobis)

Re: Updated patch for a Windows implementation of play-sound - 2

[Richard Stallman]

    >  Oh and the "normal" #ifdef style used in Emacs is:
    >
    >	#if FOO
    >	#else /* !FOO */
    >	#endif /* !FOO */

Actually our normal style is to write /* not FOO */
rather than /* !FOO */

    I find this style of #ifdef blocks to be much too cryptic for my tastes.

It is a fine way of saying what command the #endif or #else matches.
However, that is all it does.  By all means write additional comments
to give more information!

      I find that it is much
    clearer if your comments for the #else and #endif portions state exactly
    what #ifdef line they go to 

What would it mean to "state exactly"?  How can you be more explicit
than our current convention?

    I would like to thank you for testing this patch on Linux.

Actually he tested it on the GNU/Linux system.  Linux alone is just
part of a system--you can't run anything on Linux but itself.
Would you please call the system "GNU/Linux"?

See http://www.gnu.org/gnu/linux-and-gnu.html and
http://www.gnu.org/gnu/gnu-linux-faq.html.

Re: Updated patch for a Windows implementation of play-sound - 2

[Richard Stallman]

    > This is one of the few places where my coding style differs from the GNU
    > coding standards.

    I think it's better to comply with the Emacs coding standards, for
    subsequent editors probably won't update your special comments.

There is more than one good way to do these things, but inconsistency
is really ugly.  In code for Emacs, please stick to Emacs style
conventions.

RE: Updated patch for a Windows implementation of play-sound - 2

[Ben Key]

> What would it mean to "state exactly"?  How can you be more explicit
> than our current convention?

With the style of comment that I use, you can do a search for the exact text
within the comment.  This makes it much easier to find the each of the
components of the #if statement, the #if, the #else (if present), and the
#endif part.  With a comment like "!FOO" or "not FOO" you cannot search for
that exact text and find each respective part of the #if block.  This can
make the code much more difficult to read and to understand.

However, I do understand the importance of following programming standards
when programming with a large group.  I will update my code and submit a new
patch.

Since my GNU/Linux system is for the most part up and running (I still
cannot connect to it from my other computers because I have failed to set up
Samba, but I can connect to my other computers from it), I will be able to
test the patch on Windows with MinGW and MSVC 6.0 and on GNU/Linux.