[Vorbis-dev] patch for bugs in vorbis-tools-1.1.1

[Mikulas Patocka]

Hi

I've found bugs in vorbis-tools and wrote this patch to fix them:

1st part is to fix single-channel flac files.


2nd part fixes a problem with thread race causing lockup if you press 
Ctrl-C
Ctrl-C sets sig_request.cancel, buffer thread exits if it sees 
sig_request.cancel
However, if Ctrl-C was pressed here
       do {

         if (nthc-- == 0) {
----------------> Ctrl-C
           if (audio_buffer)
             buffer_submit_data(audio_buffer, convbuffer, ret);
           else
             audio_play_callback(convbuffer, ret, eos, &audio_play_arg);

           nthc = options.nth - 1;
         }
, the code gets into buffer_submit_data that will wait on buf->write_cond 
that is never signalled because the buffer thread has just exited.
The race is heavily dependent on thread scheduling and on some OSes it 
might happen very rarely or not at all.

I solved it by disabling sig_request.cancel, I think it is not needed at 
all, because buffer thread is reliably ended via buf->cancel_flag in 
buffer_thread_kill. Probably you can remove sig_request.cancel from the 
code at all.


3rd part fixes calling pthread_mutex_unlock on unlocked mutex. glibc 
probably accepts it but other thread libraries don't. Posix says that 
process behaviour when calling pthread_mutex_unlock on unlocked mutex is 
undefined and my library just prints error message and crashes in that 
case.

Mikulas


diff -u -r X/VORBIS-TOOLS-1.1.1/OGG123/FLAC_FORMAT.C VORBIS-TOOLS-1.1.1/OGG123/FLAC_FORMAT.C
--- X/VORBIS-TOOLS-1.1.1/OGG123/FLAC_FORMAT.C	2005-06-03 11:15:09.000000000 +0100
+++ VORBIS-TOOLS-1.1.1/OGG123/FLAC_FORMAT.C	2006-01-21 01:11:21.000000000 +0100
@@ -229,11 +229,11 @@
        if (audio_fmt->word_size == 1) {
  	for (i = 0; i < priv->channels; i++)
  	  for (j = 0; j < copy; j++)
-	    buf8[(j+realsamples)*2+i] = (FLAC__int8) (0xFF & priv->buf[i][j+priv->buf_start]);
+	    buf8[(j+realsamples)*audio_fmt->channels+i] = (FLAC__int8) (0xFF & priv->buf[i][j+priv->buf_start]);
        } else if (audio_fmt->word_size == 2) {
  	for (i = 0; i < priv->channels; i++)
  	  for (j = 0; j < copy; j++)
-	    buf16[(j+realsamples)*2+i] = (FLAC__int16) (0xFFFF & priv->buf[i][j+priv->buf_start]);
+	    buf16[(j+realsamples)*audio_fmt->channels+i] = (FLAC__int16) (0xFFFF & priv->buf[i][j+priv->buf_start]);
        }

        priv->buf_start += copy;
diff -u -r X/VORBIS-TOOLS-1.1.1/OGG123/OGG123.C VORBIS-TOOLS-1.1.1/OGG123/OGG123.C
--- X/VORBIS-TOOLS-1.1.1/OGG123/OGG123.C	2005-06-03 11:15:09.000000000 +0100
+++ VORBIS-TOOLS-1.1.1/OGG123/OGG123.C	2006-01-21 01:25:30.000000000 +0100
@@ -104,7 +104,7 @@
      else
        sig_request.skipfile = 1;

-    sig_request.cancel = 1;
+    /*sig_request.cancel = 1;*/
      sig_request.last_ctrl_c = now;
      break;

diff -u -r X/VORBIS-TOOLS-1.1.1/OGG123/STATUS.C VORBIS-TOOLS-1.1.1/OGG123/STATUS.C
--- X/VORBIS-TOOLS-1.1.1/OGG123/STATUS.C	2005-06-03 11:15:09.000000000 +0100
+++ VORBIS-TOOLS-1.1.1/OGG123/STATUS.C	2006-01-21 01:22:16.000000000 +0100
@@ -316,6 +316,7 @@

  void status_reset_output_lock ()
  {
+  pthread_mutex_trylock(&output_lock);
    pthread_mutex_unlock(&output_lock);
  }