[PATCH] fix lots of bugs in vls affecting TV streaming

Andrew Baumann <[email protected]> Thu, 16 Oct 2003 11:48:55 +1000

Newsgroups	gmane.comp.video.videolan.vls.devel
Message-ID	<[email protected]>

Hi,

I've been trying to get a V4L-captured TV stream out of vls for a few days 
now, and in the process I've found quite a few bugs. The attached patch:

 * changes C_Semaphore::TryWait() to accept any non-zero return value from
   sem_trywait() rather than just EAGAIN as indicating that the semaphore is
   held. The version in RedHat 9 was returning -1 rather than EAGAIN (might be
   related to the NPTL libraries, I'm not sure).

 * Puts the correct ES ID on the audio stream generated by raw2ts, rather than
   0x3 (MPEG1 audio). I had to do this to try to get AC3 working.

 * Fixes the video and audio grabbers / encoder thread to use a unique buffer
   for each frame / sample! Previously the audio grabber thread created one
   buffer, read each sample into that same buffer, and placed it into the
   fifo. This obviously isn't safe, and was causing the garbled audio that I
   was getting (and which others seem to have reported). I also noticed that
   the video code was doing the same thing, I can only imagine that it worked
   because on a fast CPU you'd be processing one frame at a time.

 * Fixes the video and audio threads not to call Release() on the condition
   variable after a Signal(), which itself already releases the lock (another
   race condition).

 * Changes the time stamping code in the audio capture thread not to use
   floating point arithmetic.

 * Fixes the V4L reader module not to open or ioctl the audio device if audio
   is muted.

 * Fixes the constructors of C_SyncFifo, C_DatedFifo, and C_NetList to
   initialise all of their instance variables.

...unfortunately after all that work I'm still having problems with the audio. 
It's no longer completely garbled, but there are discontinuities in the 
timestamps of audio samples that I can't resolve. The capture thread seems to 
block in the read() call for up to a second, even though threre is data 
available and the CPU has idle time. As a result the audio stream plays in 
VLC with parts of very sped up audio interspersed with silence.

I eventually gave up on vls and got vlc doing the streaming although 
interestingly MPEG4 video doesn't work any more (one thing that was working 
with VLS) -- the client's ffmpeg reports errors in the video stream and can't 
decode any frames, although it is the same ffmpeg version as on the server.

hope all this helps someone,
Andrew

-- 
Your flux capacitor has gone bad.

vls.diff (text/x-diff, 13.4 KB)

Index: src/core/thread.cpp
===================================================================
RCS file: /cvs/videolan/vls/src/core/thread.cpp,v
retrieving revision 1.9
diff -u -r1.9 thread.cpp
--- src/core/thread.cpp	4 Aug 2003 08:27:23 -0000	1.9
+++ src/core/thread.cpp	16 Oct 2003 01:30:42 -0000
@@ -579,9 +579,11 @@
 {
 #ifdef PTHREAD_COND_T_IN_PTHREAD_H
 # ifdef USE_SEM_T
-  int result = sem_trywait(&sSemaphore); // Returns EAGAIN if semaphore is already present
-  if (result == EAGAIN)
+  int result = sem_trywait(&sSemaphore);
+  if (result != 0)
     return -1;
+  else
+    return 0;
 # else
   int result = pthread_mutex_trylock(&m_sMutex);
   if (result == EBUSY)
Index: src/modules/raw2ts/raw2ts.cpp
===================================================================
RCS file: /cvs/videolan/vls/src/modules/raw2ts/raw2ts.cpp,v
retrieving revision 1.36
diff -u -r1.36 raw2ts.cpp
--- src/modules/raw2ts/raw2ts.cpp	20 Aug 2003 17:43:47 -0000	1.36
+++ src/modules/raw2ts/raw2ts.cpp	16 Oct 2003 01:30:42 -0000
@@ -311,7 +311,26 @@
 
   m_cPmt = new C_DvbPsiPmt(0x1212, 0, true, 0xE0, 0x12);
   dvbpsi_PMTAddES(m_cPmt->GetLowLevelPmt(), m_type, 0xE0);
-  if (!m_bMute) dvbpsi_PMTAddES(m_cPmt->GetLowLevelPmt(), 3, 0xC0);
+  if (!m_bMute)
+  {
+    switch (m_AudioCodec->id) {
+      case CODEC_ID_MP2:
+      case CODEC_ID_MP3LAME:
+        m_atype = TS_TYPE_MPEG1_AUDIO;
+        break;
+      case CODEC_ID_AC3:
+        m_atype = 0x81; /* should be TS_TYPE_AC3 */
+        break;
+      case CODEC_ID_VORBIS:
+        /* FIXME: what is the stream type for Vorbis? */
+      default:
+        fprintf(stderr, "Warning: unknown FFmpeg audio codec %d, using bogus ES"
+                " stream type of MPEG1 audio\n", m_AudioCodec->id);
+        m_atype = TS_TYPE_MPEG1_AUDIO;
+        break;
+    }
+    dvbpsi_PMTAddES(m_cPmt->GetLowLevelPmt(), m_atype, 0xC0);
+  }
   m_cPmt->Generate();
   m_cPmt->TsReset();
 
@@ -368,29 +387,17 @@
   {
     int outSize;
 
-//    if (m_pDatedBuffer == NULL)
-    {
-      m_pDatedBuffer = m_pAudioFifo->Pop();
+    m_pDatedBuffer = m_pAudioFifo->Pop();
 
-      // encode the buffer
-      outSize = avcodec_encode_audio( m_AudioCodecContext, m_AudioOutBuf,
-                                      m_pDatedBuffer->m_iSize,
-                                      (short*)m_pDatedBuffer->m_pData );
+    // encode the buffer
+    outSize = avcodec_encode_audio( m_AudioCodecContext, m_AudioOutBuf,
+                                    m_pDatedBuffer->m_iSize,
+                                    (short*)m_pDatedBuffer->m_pData );
 
-    }
-    
     // build TS packets and send them
     m_iPos = EStoTS( pPackets, m_AudioOutBuf, outSize,
                   m_pDatedBuffer->m_TimeStamp,0xC0, &iAudioContinuityCounter);
-
-    // jpsaman, what was this code you added for ? -- tooney
-   /*    if (m_iPos >= m_pDatedBuffer->m_iSize)
-    {
-      delete m_pDatedBuffer;
-      m_pDatedBuffer = NULL;
-      m_iPos = 0;
-    }*/
-    
+    delete m_pDatedBuffer;
   }
   else
   {
@@ -438,12 +445,7 @@
              m_pDatedBuffer->m_TimeStamp, 0xE0, &iVideoContinuityCounter );
 
     if( to_free ) free( to_free ); // free the deinterlace picture
-/*    if (m_iPos >= m_pDatedBuffer->m_iSize)
-    {
-      delete m_pDatedBuffer; // free the original picture
-      m_pDatedBuffer = NULL;
-      m_iPos = 0;
-    }*/
+    delete m_pDatedBuffer; // free the original picture
   }
 
   // send some PAT and PMT !
Index: src/modules/raw2ts/raw2ts.h
===================================================================
RCS file: /cvs/videolan/vls/src/modules/raw2ts/raw2ts.h,v
retrieving revision 1.22
diff -u -r1.22 raw2ts.h
--- src/modules/raw2ts/raw2ts.h	11 Aug 2003 16:57:50 -0000	1.22
+++ src/modules/raw2ts/raw2ts.h	16 Oct 2003 01:30:42 -0000
@@ -104,6 +104,7 @@
   int m_channel;
   int m_audiofreq;
   float m_quality;
+  int m_atype;
 };
 
 
Index: src/modules/raw2ts/rawcapture.cpp
===================================================================
RCS file: /cvs/videolan/vls/src/modules/raw2ts/rawcapture.cpp,v
retrieving revision 1.2
diff -u -r1.2 rawcapture.cpp
--- src/modules/raw2ts/rawcapture.cpp	20 Aug 2003 17:43:47 -0000	1.2
+++ src/modules/raw2ts/rawcapture.cpp	16 Oct 2003 01:30:42 -0000
@@ -94,17 +94,15 @@
 {
   C_DatedBuffer * pDatedBuffer;
 
-  pDatedBuffer = new C_DatedBuffer();
-
-  if( !pDatedBuffer->Alloc( m_iPictureBufSize ) )
-  {
-    fprintf( stderr, "Error : cannot Alloc() (%d bytes)\n",
-                     m_iPictureBufSize );
-    return;
-  }
-    
   while( !m_bStop )
   {
+    pDatedBuffer = new C_DatedBuffer();
+    if( !pDatedBuffer->Alloc( m_iPictureBufSize ) )
+    {
+      fprintf( stderr, "Error : cannot Alloc() (%d bytes)\n",
+                       m_iPictureBufSize );
+      return;
+    }
 
     // read data from the device; this is the blocking stage.
     byte * tmp = NULL;
@@ -128,7 +126,6 @@
     // warn the encoder thread we have something
     m_pCondition->Protect();
     m_pCondition->Signal();
-    m_pCondition->Release();
   }
 }
 
@@ -175,19 +172,22 @@
   struct audio_buf_info buf_info;
   int delay;
 
-  pDatedBuffer = new C_DatedBuffer();
-
-  if( !pDatedBuffer->Alloc( m_iAudioSampleBufSize ) )
-  {
-    fprintf( stderr, "Error : cannot Alloc() (%d bytes)\n",
-                     m_iAudioSampleBufSize );
-    return;
-  }
-
   while( !m_bStop )
   {
+    pDatedBuffer = new C_DatedBuffer();
+    if( !pDatedBuffer->Alloc( m_iAudioSampleBufSize ) )
+    {
+      fprintf( stderr, "Error : cannot Alloc() (%d bytes)\n",
+                       m_iAudioSampleBufSize );
+      return;
+    }
+
     // read data from the device; this is the blocking stage
     delay = read( m_iAudioFD, pDatedBuffer->m_pData, m_iAudioSampleBufSize );
+    if (delay <= 0) {
+      perror("Error reading audio");
+      return;
+    }
 
     // date it
     currentDate = GetTime();
@@ -195,24 +195,25 @@
     // correct the date because of kernel buffering
     if( ioctl( m_iAudioFD, SNDCTL_DSP_GETISPACE, &buf_info ) == 0 )
       delay += buf_info.bytes;
-    currentDate -= (u64)( 1000000 * (float)delay /
-                          ( 2.0     /* 16 bits samples */
-                            * m_channel     /* channel */
-                            * m_audiofreq /* sample rate */ ) );
 
+    currentDate -= (u64)1000000 * (u64)delay
+                   / 2            /* 16 bit samples */
+                   / m_channel    /* channel */
+                   / m_audiofreq; /* sample rate */
     pDatedBuffer->m_TimeStamp = ( currentDate * 9 / 100 ) &  0x0effffff;
 
     // add it to fifo
     int iRc = m_pFifo->Push( pDatedBuffer );
-    if (iRc)
+    if (iRc) {
       fprintf( stderr, "Warning : trashing an audio frame (%lld)... "
                        "your computer is too slow !\n",
                         pDatedBuffer->m_TimeStamp );
+      delete pDatedBuffer;
+    }
 
     // warn the encoder thread we have something
     m_pCondition->Protect();
     m_pCondition->Signal();
-    m_pCondition->Release();
   }
 }
 
Index: src/modules/v4lreader/v4lreader.cpp
===================================================================
RCS file: /cvs/videolan/vls/src/modules/v4lreader/v4lreader.cpp,v
retrieving revision 1.19
diff -u -r1.19 v4lreader.cpp
--- src/modules/v4lreader/v4lreader.cpp	20 Aug 2003 17:43:47 -0000	1.19
+++ src/modules/v4lreader/v4lreader.cpp	16 Oct 2003 01:30:42 -0000
@@ -99,6 +99,7 @@
 {
   m_pBroadcast = pBroadcast;
   m_strDeviceName = pBroadcast->GetOption("v4l_device");
+  m_bMute = pBroadcast->GetOption("v4l_mute") == "true" ? true : false;
   m_strAudioDeviceName = pBroadcast->GetOption("v4l_audiodevice");
   
   m_iCurrentChannel = pBroadcast->GetOption("v4l_channel").ToInt();
@@ -355,69 +356,74 @@
         m_pBroadcast->SetOption("Palette",PIX_FMT_YUV420P);
         break;
   }
+
   /* sound stuff now */
-  
-  /* open the device */
-  if( ( m_iAudioFD = open(m_strAudioDeviceName.GetString(), O_RDONLY ) ) < 0 )
-  {
-    Log(m_hLog, LOG_ERROR, "could not open " +
-                        m_strAudioDeviceName+" for reading\n");
-    return;
-  }
-  else
-    Log(m_hLog, LOG_NOTE, "Info : Opening sound device " +
-                    m_strAudioDeviceName);
-  
-  /* 16 bits samples, little endian */
-  int format = AFMT_S16_LE;
-  if( ioctl( m_iAudioFD, SNDCTL_DSP_SETFMT, &format ) < 0 )
-  {
-    Log(m_hLog, LOG_ERROR, "ioctl SNDCTL_DSP_SETFMT failed\n" );
-    return;
-  }
-  if( format != AFMT_S16_LE )
+  if (!m_bMute)
   {
-    Log(m_hLog, LOG_ERROR, "AFMT_S16_LE not supported\n" );
-    return;
-  }
+    /* open the device */
+    if( ( m_iAudioFD = open(m_strAudioDeviceName.GetString(), O_RDONLY ) ) < 0 )
+    {
+      Log(m_hLog, LOG_ERROR, "could not open " +
+                          m_strAudioDeviceName+" for reading\n");
+      return;
+    }
+    else
+      Log(m_hLog, LOG_NOTE, "Info : Opening sound device " +
+                      m_strAudioDeviceName);
 
-  /* channel count */
-  int stereo = m_iChannel != 1 ;
-  if( ioctl( m_iAudioFD, SNDCTL_DSP_STEREO, &stereo ) < 0 )
-  {
-    fprintf( stderr, "ioctl SNDCTL_DSP_STEREO (%d) failed : %s\n", stereo, strerror(errno));
-    fprintf( stderr, "Fall back to mono\n");
-
-    /* Back to mono, and update value (raw2ts will catch it later) */
-    m_iChannel = 1;
-    m_pBroadcast->SetOption("v4l_audiochannel","1");
+    /* 16 bits samples, little endian */
+    int format = AFMT_S16_LE;
+    if( ioctl( m_iAudioFD, SNDCTL_DSP_SETFMT, &format ) < 0 )
+    {
+      Log(m_hLog, LOG_ERROR, "ioctl SNDCTL_DSP_SETFMT failed\n" );
+      return;
+    }
+    if( format != AFMT_S16_LE )
+    {
+      Log(m_hLog, LOG_ERROR, "AFMT_S16_LE not supported\n" );
+      return;
+    }
 
-    stereo = 0;
+    /* channel count */
+    int stereo = m_iChannel != 1 ;
     if( ioctl( m_iAudioFD, SNDCTL_DSP_STEREO, &stereo ) < 0 )
     {
       fprintf( stderr, "ioctl SNDCTL_DSP_STEREO (%d) failed : %s\n", stereo, strerror(errno));
-    }
+      fprintf( stderr, "Fall back to mono\n");
 
-  }
+      /* Back to mono, and update value (raw2ts will catch it later) */
+      m_iChannel = 1;
+      m_pBroadcast->SetOption("v4l_audiochannel","1");
 
-  /* sample rate */
-  int m_iSample_rate ;
-  if( ioctl( m_iAudioFD, SNDCTL_DSP_SPEED, &m_iSample_rate ) < 0 )
-  {
-    fprintf( stderr, "ioctl SNDCTL_DSP_SPEED failed (%d) : %s\n",
-                                                  m_iSample_rate, strerror(errno));
-    fprintf( stderr, "back to 44100 Hz\n");
-
-    /* Back to 44110 Hz, and update value (raw2ts will catch it later) */
-    m_iSample_rate = 44100;
-    m_pBroadcast->SetOption("v4l_audiofreq","44100");
+      stereo = 0;
+      if( ioctl( m_iAudioFD, SNDCTL_DSP_STEREO, &stereo ) < 0 )
+      {
+	fprintf( stderr, "ioctl SNDCTL_DSP_STEREO (%d) failed : %s\n", stereo, strerror(errno));
+      }
 
+    }
+
+    /* sample rate */
+    int m_iSample_rate ;
     if( ioctl( m_iAudioFD, SNDCTL_DSP_SPEED, &m_iSample_rate ) < 0 )
     {
       fprintf( stderr, "ioctl SNDCTL_DSP_SPEED failed (%d) : %s\n",
                                                     m_iSample_rate, strerror(errno));
+      fprintf( stderr, "back to 44100 Hz\n");
+
+      /* Back to 44110 Hz, and update value (raw2ts will catch it later) */
+      m_iSample_rate = 44100;
+      m_pBroadcast->SetOption("v4l_audiofreq","44100");
+
+      if( ioctl( m_iAudioFD, SNDCTL_DSP_SPEED, &m_iSample_rate ) < 0 )
+      {
+	fprintf( stderr, "ioctl SNDCTL_DSP_SPEED failed (%d) : %s\n",
+                                                      m_iSample_rate, strerror(errno));
+      }
+
     }
-    
+  } else {
+    m_iAudioFD = -1;
   }
   
   m_pAudioBuffer = NULL;
Index: src/modules/v4lreader/v4lreader.h
===================================================================
RCS file: /cvs/videolan/vls/src/modules/v4lreader/v4lreader.h,v
retrieving revision 1.11
diff -u -r1.11 v4lreader.h
--- src/modules/v4lreader/v4lreader.h	30 Jun 2003 21:37:05 -0000	1.11
+++ src/modules/v4lreader/v4lreader.h	16 Oct 2003 01:30:42 -0000
@@ -96,6 +96,7 @@
   byte * m_pPictureBuffer; // Pointer to the picture buffer
 
   // variables for dsp reading
+  bool m_bMute;
   C_String m_strAudioDeviceName;
   int m_iAudioFD; // file descriptor
   byte * m_pAudioBuffer;
Index: src/server/buffer.cpp
===================================================================
RCS file: /cvs/videolan/vls/src/server/buffer.cpp,v
retrieving revision 1.11
diff -u -r1.11 buffer.cpp
--- src/server/buffer.cpp	14 Aug 2003 17:20:36 -0000	1.11
+++ src/server/buffer.cpp	16 Oct 2003 01:30:42 -0000
@@ -52,6 +52,7 @@
 //------------------------------------------------------------------------------
 C_SyncFifo::C_SyncFifo(unsigned int iSize) : m_cNotEmptySignal(0),
                                              m_cNotFullSignal(iSize),
+                                             m_cPrefilling(),
                                              m_cFifo(iSize)
 {
   ASSERT(iSize > 1);
@@ -208,7 +209,8 @@
 //------------------------------------------------------------------------------
 // Constructor
 //------------------------------------------------------------------------------
-C_DatedFifo::C_DatedFifo(unsigned int uiSize) : m_cFifo(uiSize, YES, true)
+C_DatedFifo::C_DatedFifo(unsigned int uiSize) : m_cFifo(uiSize, YES, true),
+                                                m_cMutex()
 {
   ASSERT(uiSize > 0);
 }
@@ -255,7 +257,7 @@
 //------------------------------------------------------------------------------
 // 
 //------------------------------------------------------------------------------
-C_NetList::C_NetList(unsigned int iSize/* = 65536*/)
+C_NetList::C_NetList(unsigned int iSize/* = 65536*/) : m_sMutex()
 {
   iBuffSize = iSize;