just now i realized the wrong underlying assumption i did in metronom: fixing

audio drift the same way i did for video doesn't work, since sound card
won't play it slower or faster because of the vpts's.
with this patch i hope to have definitely separated the 2 sources of
audio drift: (1) nominal sample rate discrepancy to stream pts,
(2) sound card drift to system clock.
- revert feedback audio loop
- updated metronom's audio drift
- always do a full gap filling if needed

CVS patchset: 3392
CVS date: 2002/11/30 22:09:42

3 files changed, 68 insertions, 34 deletions

diff --git a/src/xine-engine/audio_out.c b/src/xine-engine/audio_out.c
index 40abd78cb..935e8b671 100644
--- a/src/xine-engine/audio_out.c
+++ b/src/xine-engine/audio_out.c
@@ -17,7 +17,7 @@
  * along with self program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA
  *
- * $Id: audio_out.c,v 1.84 2002/11/27 23:13:09 heikos Exp $
+ * $Id: audio_out.c,v 1.85 2002/11/30 22:09:42 miguelfreitas Exp $
  * 
  * 22-8-2001 James imported some useful AC3 sections from the previous alsa driver.
  *   (c) 2001 Andy Lo A Foe <andy@alsaplayer.org>
@@ -80,6 +80,32 @@
 
 #define ZERO_BUF_SIZE         5000
 
+/* By adding gap errors (difference between reported and expected
+ * sound card clock) into metronom's vpts_offset we can use its 
+ * smoothing algorithms to correct sound card clock drifts.
+ * obs: previously this error was added to xine scr.
+ *
+ * audio buf ---> metronom --> audio fifo --> (buf->vpts - hw_vpts)
+ *           (vpts_offset + error)                     gap
+ *                    <---------- control --------------|
+ *
+ * Unfortunately audio fifo adds a large delay to our closed loop.
+ *
+ * These are designed to avoid updating the metronom too fast.
+ * - it will only be updated 1 time per second (so it has a chance of
+ *   distributing the error for several frames).
+ * - it will only be updated 2 times for the whole audio fifo size
+ *   length (so the control will wait to see the feedback effect)
+ * - each update will be of gap/SYNC_GAP_RATE.
+ *
+ * Sound card clock correction can only provide smooth playback for
+ * errors < 1% nominal rate. For bigger errors (bad streams) audio
+ * buffers may be dropped or gaps filled with silence.
+ */
+#define SYNC_TIME_INVERVAL  (1 * 90000)
+#define SYNC_BUF_INTERVAL   NUM_AUDIO_BUFFERS / 2
+#define SYNC_GAP_RATE       4
+
 struct audio_fifo_s {
   audio_buffer_t    *first;
   audio_buffer_t    *last;
@@ -521,7 +547,6 @@ static void *ao_loop (void *this_gen) {
 	}
       }
 
-      this->allow_full_ao_fill_gap = 1;
 #ifdef LOG
       printf ("audio_out:loop:pause: I feel sleepy.\n");
 #endif
@@ -566,7 +591,7 @@ static void *ao_loop (void *this_gen) {
     gap = in_buf->vpts - hw_vpts;
 #ifdef LOG
     printf ("audio_out: hw_vpts : %lld   buffer_vpts : %lld   gap : %lld\n",
-	    hw_vpts, in_buf->vpts, gap);
+	    hw_vpts, buf->vpts, gap);
 #endif
 
     /*
@@ -581,24 +606,20 @@ static void *ao_loop (void *this_gen) {
       printf ("audio_out:loop: next fifo\n");
 #endif
       fifo_append (this->free_fifo, in_buf);
+      in_buf = NULL;
 
 #ifdef LOG
       printf ("audio_out: audio package (vpts = %lld, gap = %lld) dropped\n", 
-	      in_buf->vpts, gap);
+	      buf->vpts, gap);
 #endif
 
-      in_buf = NULL;
-
       
-      /*
-       * for small gaps ( tolerance < abs(gap) < AO_MAX_GAP ) the metronom's
-       * clock is set to compensate.
-       *
-       * hopefully this will guarantee smooth audio playback - at the possible
-       * expense of the video running a little bit faster or slower than it is
-       * supposed to.
+      /* for small gaps ( tolerance < abs(gap) < AO_MAX_GAP ) 
+       * feedback them into metronom's vpts_offset. 
        */
-    } else if ( (abs(gap) < AO_MAX_GAP) && (abs(gap) > this->gap_tolerance) ) {
+    } else if ( abs(gap) < AO_MAX_GAP && abs(gap) > this->gap_tolerance &&
+           cur_time > (last_sync_time + SYNC_TIME_INVERVAL) && 
+           bufs_since_sync >= SYNC_BUF_INTERVAL ) {
 	xine_stream_t *stream;
 #ifdef LOG
         printf ("audio_out: audio_loop: ADJ_VPTS\n"); 
@@ -606,9 +627,8 @@ static void *ao_loop (void *this_gen) {
 	pthread_mutex_lock(&this->streams_lock);
 	for (stream = xine_list_first_content(this->streams); stream;
 	     stream = xine_list_next_content(this->streams)) {
-	  int64_t curtime = stream->metronom->clock->get_current_time(stream->metronom->clock);
-	  stream->metronom->clock->adjust_clock(stream->metronom->clock,
-						curtime + gap);
+	  stream->metronom->set_option(stream->metronom, METRONOM_ADJ_VPTS_OFFSET,
+                                       -gap/SYNC_GAP_RATE );
           last_sync_time = cur_time;
           bufs_since_sync = 0;
 	}
@@ -616,12 +636,7 @@ static void *ao_loop (void *this_gen) {
 
     } else if ( gap > AO_MAX_GAP ) {
       /* for big gaps output silence */
-      if (this->allow_full_ao_fill_gap) {
-        ao_fill_gap (this, gap);
-        this->allow_full_ao_fill_gap = 0;
-      } else {
-        ao_fill_gap (this, gap / 2);
-      }
+      ao_fill_gap (this, gap);
     } else {
 #if 0
       {
@@ -828,7 +843,7 @@ static void ao_put_buffer (xine_audio_port_t *this, audio_buffer_t *buf, xine_st
 	  pts, buf->vpts);
 #endif
 
-  if ( buf->vpts<this->last_audio_vpts) {
+  if ( buf->vpts + AO_MAX_GAP < this->last_audio_vpts) {
 
     /* reject buffer */
     printf ("audio_out: rejected buffer vpts=%lld, last_audio_vpts=%lld\n", 
@@ -1027,7 +1042,6 @@ static void ao_flush (xine_audio_port_t *this) {
   fifo_append_int (this->out_fifo, buf);
 
   this->flush_audio_driver = 1;
-  this->allow_full_ao_fill_gap = 1;
   pthread_mutex_unlock (&this->free_fifo->mutex);
   pthread_mutex_unlock (&this->out_fifo->mutex);
 }
@@ -1062,7 +1076,6 @@ xine_audio_port_t *ao_new_port (xine_t *xine, ao_driver_t *driver) {
   this->audio_loop_running     = 0;
   this->audio_paused           = 0;
   this->flush_audio_driver     = 0;
-  this->allow_full_ao_fill_gap = 0;
   this->zero_space             = xine_xmalloc (ZERO_BUF_SIZE * 2 * 6);
   this->gap_tolerance          = driver->get_gap_tolerance (this->driver);
 
diff --git a/src/xine-engine/audio_out.h b/src/xine-engine/audio_out.h
index 6fa21b531..c68fc9d94 100644
--- a/src/xine-engine/audio_out.h
+++ b/src/xine-engine/audio_out.h
@@ -17,7 +17,7 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA
  *
- * $Id: audio_out.h,v 1.42 2002/11/20 13:42:41 esnel Exp $
+ * $Id: audio_out.h,v 1.43 2002/11/30 22:09:42 miguelfreitas Exp $
  */
 #ifndef HAVE_AUDIO_OUT_H
 #define HAVE_AUDIO_OUT_H
@@ -241,7 +241,6 @@ struct xine_audio_port_s {
 
   int64_t         passthrough_offset;
   int             flush_audio_driver;
-  int             allow_full_ao_fill_gap;
 
   int             do_compress;
   double          compression_factor;   /* current compression */
diff --git a/src/xine-engine/metronom.c b/src/xine-engine/metronom.c
index 539113a8f..d362cf8be 100644
--- a/src/xine-engine/metronom.c
+++ b/src/xine-engine/metronom.c
@@ -17,7 +17,7 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA
  *
- * $Id: metronom.c,v 1.104 2002/11/27 21:41:11 heikos Exp $
+ * $Id: metronom.c,v 1.105 2002/11/30 22:09:42 miguelfreitas Exp $
  */
 
 #ifdef HAVE_CONFIG_H
@@ -49,6 +49,8 @@
 #define AUDIO_DRIFT_TOLERANCE 45000
 #define AV_DIFF_TOLERANCE     45000
 
+/*#define OLD_DRIFT_CORRECTION  1*/
+
 /* redefine abs as macro to handle 64-bit diffs.
    i guess llabs may not be available everywhere */
 #define abs(x) ( ((x)<0) ? -(x) : (x) )
@@ -282,8 +284,6 @@ static int64_t metronom_got_spu_packet (metronom_t *this, int64_t pts) {
 
 static void metronom_handle_video_discontinuity (metronom_t *this, int type,
 						 int64_t disc_off) {
-  int64_t diff;
-
   pthread_mutex_lock (&this->lock);
 
   this->video_discontinuity_count++;
@@ -536,10 +536,22 @@ static int64_t metronom_got_audio_samples (metronom_t *this, int64_t pts,
   }
   vpts = this->audio_vpts;
 
+  /* drift here is caused by streams where nominal sample rate differs from 
+   * the rate of which pts increments. fixing the audio_vpts won't do us any
+   * good because sound card won't play it faster or slower just because
+   * we want. however, adding the error to the vpts_offset will force video
+   * to change it's frame rate to keep in sync with us.
+   */
+#if OLD_DRIFT_CORRECTION
   this->audio_vpts += nsamples * (this->pts_per_smpls-this->audio_drift_step)
                       / AUDIO_SAMPLE_NUM;
   this->audio_samples += nsamples;
-  
+#else
+  this->audio_vpts += nsamples * this->pts_per_smpls / AUDIO_SAMPLE_NUM;
+  this->audio_samples += nsamples;
+  this->vpts_offset += nsamples * this->audio_drift_step / AUDIO_SAMPLE_NUM;
+#endif                 
+                        
 #ifdef LOG
   printf ("metronom: audio vpts for %10lld : %10lld\n", pts, vpts);
 #endif
@@ -559,10 +571,20 @@ static void metronom_set_option (metronom_t *this, int option, int64_t value) {
     printf ("metronom: av_offset=%lld pts\n", this->av_offset);
     break;
   case METRONOM_ADJ_VPTS_OFFSET:
+#if OLD_DRIFT_CORRECTION
     this->vpts_offset += value;
-#ifdef LOG
-    printf ("metronom: adjusting vpts_offset by %lld\n", value );
+#else
+    this->audio_vpts += value;
 #endif
+
+/*#ifdef LOG*/
+    /* that message should be rare, please report otherwise.
+     * when xine is in some sort of "steady state" hearing it
+     * once in a while means a small sound card drift (or system
+     * clock drift -- who knows?). nothing to worry about.
+     */
+    printf ("metronom: fixing sound card drift by %lld pts\n", value );
+/*#endif*/
     break;
   default:
     printf ("metronom: unknown option in set_option: %d\n",