Timeline: More locking fixes. Also, build peaks cache levels in threads launched by UI thread, instead of in the record diskthreads.

timeline/src/Audio_Region.C

@@ -618,13 +618,12 @@ Audio_Region::draw ( void )
                     oend = end;
                 }
             }
-            else
+            
+            if ( _clip->peaks()->needs_more_peaks() && ! transport->rolling )
             {
-                if ( ! transport->rolling )
+                    /* maybe create a thread to make the peaks */
-                {
+                    /* this function will just return if there's nothing to do. */
-                    /* create a thread to make the peaks */
                     _clip->peaks()->make_peaks_asynchronously( Audio_Region::peaks_ready_callback, this );
-                }
             }
         }
         else

timeline/src/Engine/Disk_Stream.C

@@ -74,7 +74,7 @@ Disk_Stream::~Disk_Stream ( )
 {
     /* it isn't safe to do all this with the RT thread running */
 
-    timeline->wrlock();
+//    timeline->wrlock();
 
     shutdown();
 
@@ -85,7 +85,7 @@ Disk_Stream::~Disk_Stream ( )
     for ( int i = channels(); i--; )
         jack_ringbuffer_free( _rb[ i ] );
 
-    timeline->unlock();
+//    timeline->unlock();
 }
 
 
@@ -144,13 +144,15 @@ Disk_Stream::shutdown ( void )
 {
     if ( _thread.running() )
     {
+        DMESSAGE( "Sending terminate signal to diskthread." );
+
         _terminate = true;
         
         /* try to wake the thread so it'll see that it's time to die */
         while ( _terminate )
         {
-            usleep( 100 );
             block_processed();
+            usleep( 1000 );
         }
         
         _thread.join();

timeline/src/Engine/Peaks.C

@@ -104,9 +104,7 @@ class Peakfile
     FILE *_fp;
     nframes_t _chunksize;
     int _channels;   /* number of channels this peakfile represents */
-//    nframes_t _length; /* length, in frames, of the clip this peakfile represents */
     off_t _offset;
-//    int _blocks;
 
     struct block_descriptor
     {
@@ -127,14 +125,17 @@ class Peakfile
 
 public:
 
+    int nblocks ( void ) const 
+    {
+        return blocks.size();
+    }
+
     Peakfile ( )
         {
-//            _blocks = 0;
             _fp = NULL;
             _offset = 0;
             _chunksize = 0;
             _channels = 0;
-//            _length = 0;
         }
 
     ~Peakfile ( )
@@ -143,6 +144,11 @@ public:
                 close();
         }
 
+    void rescan ( void )
+        {
+            blocks.clear();
+        }
+
     /* int blocks ( void ) const { return blocks.size(); } */
     /** find the best block for /chunksize/ */
     void
@@ -184,9 +190,6 @@ public:
             if ( ! blocks.size() )
                 FATAL( "Peak file contains no blocks!" );
 
-
-//            DMESSAGE( "peakfile has %d blocks.", blocks.size() );
-
             blocks.sort();
 
             /* fall back on the smallest chunksize */
@@ -204,7 +207,6 @@ public:
                 }
 
 //           DMESSAGE( "using peakfile block for chunksize %lu", _chunksize );
-//            _blocks = blocks.size();
             _offset = ftello( _fp );
         }
 
@@ -239,6 +241,7 @@ public:
     bool
     open ( const char *name, int channels, nframes_t chunksize )
         {
+            assert( ! _fp );
 //            _chunksize = 0;
             _channels = channels;
 
@@ -263,6 +266,8 @@ public:
     bool
     open ( FILE *fp, int channels, nframes_t chunksize )
         {
+            assert( ! _fp );
+
             _fp = fp;
             _chunksize = 0;
             _channels = channels;
@@ -364,7 +369,9 @@ public:
 
 Peaks::Peaks ( Audio_File *c )
 {
-    _pending = false;
+    _rescan_needed = false;
+    _first_block_pending = false;
+    _mipmaps_pending = false;
     _clip = c;
     _peak_writer = NULL;
     _peakfile = new Peakfile();
@@ -412,19 +419,33 @@ Peaks::ready ( nframes_t s, nframes_t npeaks, nframes_t chunksize ) const
 bool
 Peaks::peakfile_ready ( void ) const
 {
-    return current() && ! _pending;
+    if ( _rescan_needed )
+    {
+        DMESSAGE( "Rescanning peakfile" );
+        _peakfile->rescan();
+        if ( _peakfile->open( _clip->filename(), _clip->channels(), 256 ) )
+            _peakfile->close();
+
+        _rescan_needed = false;
+    }
+
+    return current() && ! _first_block_pending;
 }
 
+/** start building peaks and/or peak mipmap in another thread. It is
+ * safe to call this again before the thread finishes. /callback/ will
+ * be called with /userdata/ FROM THE PEAK BUILDING THREAD when the
+ * peaks are finished.  */
 void
 Peaks::make_peaks_asynchronously ( void(*callback)(void*), void *userdata ) const
 {
     /* already working on it... */
-    if( _pending )
+    if( _first_block_pending || _mipmaps_pending )
         return;
     
-//    make_peaks();
+    /* maybe still building mipmaps... */
-
+    _first_block_pending = _peakfile->nblocks() < 1;
-    _pending = true;
+    _mipmaps_pending = _peakfile->nblocks() <= 1;
     
     peak_thread_data *pd = new peak_thread_data();
     
@@ -434,17 +455,14 @@ Peaks::make_peaks_asynchronously ( void(*callback)(void*), void *userdata ) cons
     
     _make_peaks_thread.clone( &Peaks::make_peaks, pd );
     _make_peaks_thread.detach();
+    
+    DMESSAGE( "Starting new peak building thread" );
 }
 
 nframes_t
 Peaks::read_peakfile_peaks ( Peak *peaks, nframes_t s, nframes_t npeaks, nframes_t chunksize ) const
 {
-    /* if ( _pending ) */
+    if ( ! _peakfile->open( _clip->filename(), _clip->channels(), chunksize ) )
-    /*     return 0; */
-
-//    Peakfile _peakfile;
-
-    if ( ! _peakfile->open( _clip->filename(),  _clip->channels(), chunksize ) )
     {
         DMESSAGE( "Failed to open peakfile!" );
         return 0;
@@ -545,7 +563,7 @@ Peaks::current ( void ) const
 {
     char *pn = peakname( _clip->filename() );
 
-    bool b = ! newer( _clip->filename(), pn );
+    bool b = newer( pn, _clip->filename() );
 
     free( pn );
 
@@ -558,45 +576,38 @@ Peaks::make_peaks ( void *v )
 {
     peak_thread_data *pd = (peak_thread_data*)v;
 
-    pd->peaks->make_peaks();
+    if ( pd->peaks->make_peaks() )
+    {
+        if ( pd->callback )
+            pd->callback( pd->userdata );
 
-    if ( pd->callback )
+        pd->peaks->_rescan_needed = true;
-        pd->callback( pd->userdata );
+    }
     
     delete pd;
 
     return NULL;
 }
 
+bool
+Peaks::needs_more_peaks ( void ) const
+{
+    return _peakfile->nblocks() <= 1 && ! ( _first_block_pending || _mipmaps_pending );
+}
+
 bool
 Peaks::make_peaks ( void ) const
 {
     Peaks::Builder pb( this );
 
+    /* make the first block */
     int b = pb.make_peaks();
     
-    _pending = false;
+    _first_block_pending = false;
 
-    return b;
+    b = pb.make_peaks_mipmap();
-}
 
-/* thread entry point */
+    _mipmaps_pending = false;
-void *
-Peaks::make_peaks_mipmap ( void *v )
-{
-    ((Peaks*)v)->make_peaks_mipmap();
-
-    return NULL;
-}
-
-bool
-Peaks::make_peaks_mipmap ( void ) const
-{
-    Peaks::Builder pb( this );
-
-    bool b = pb.make_peaks_mipmap();
-
-    _pending = false;
 
     return b;
 }
@@ -638,8 +649,6 @@ Peaks::finish_writing ( void )
 
     delete _peak_writer;
     _peak_writer = NULL;
-
-    make_peaks_mipmap();
 }
 
 void
@@ -792,15 +801,12 @@ Peaks::Builder::write_block_header ( nframes_t chunksize )
     fflush( fp );
 }
 
-
-
-
 /** generate additional cache levels for a peakfile with only 1 block (ie. that of a new capture) */
 bool
 Peaks::Builder::make_peaks_mipmap ( void )
 {
     if ( ! Peaks::mipmapped_peakfiles )
-        return true;
+        return false;
 
     Audio_File *_clip = _peaks->_clip;
 
@@ -895,10 +901,7 @@ Peaks::Builder::make_peaks_mipmap ( void )
         }
         while ( len > 0 && s < _clip->length() );
 
-        DMESSAGE( "Last sample was %lu", s );
+        DMESSAGE( "Last sample was %lu", (unsigned long)s );
-
-        /* fflush( fp ); */
-        /* fsync( fileno( fp ) ); */
 
         pf.leave_open();
     }
@@ -918,43 +921,46 @@ Peaks::Builder::make_peaks ( void )
 
     const char *filename = _clip->filename();
 
-    DMESSAGE( "building peaks for \"%s\"", filename );
+    if ( _peaks->_peakfile && _peaks->_peakfile->nblocks() > 1 )
-
-    char *pn = peakname( filename );
-
-    if ( ! ( fp  = fopen( pn, "w+" ) ) )
     {
-        free( pn );
+        /* this peakfile already has enough blocks */
         return false;
     }
+    else
+    {
+        DMESSAGE( "building peaks for \"%s\"", filename );
         
-    free( pn );
+        char *pn = peakname( filename );
         
-    _clip->seek( 0 );
+        if ( ! ( fp  = fopen( pn, "w+" ) ) )
+        {
+            free( pn );
+            return false;
+        }
         
-    Peak buf[ _clip->channels() ];
+        free( pn );
         
-    DMESSAGE( "building level 1 peak cache" );
+        _clip->seek( 0 );
         
-    write_block_header( Peaks::cache_minimum );
+        Peak buf[ _clip->channels() ];
         
-    /* build first level from source */
+        DMESSAGE( "building level 1 peak cache" );
-    off_t len;
-    do {
-        len = _peaks->read_source_peaks( buf, 1, Peaks::cache_minimum );
         
-        fwrite( buf, sizeof( buf ), len, fp );
+        write_block_header( Peaks::cache_minimum );
+        
+        /* build first level from source */
+        off_t len;
+        do {
+            len = _peaks->read_source_peaks( buf, 1, Peaks::cache_minimum );
+            
+            fwrite( buf, sizeof( buf ), len, fp );
+        }
+        while ( len );
+        
+        fclose( fp );
+
+        DMESSAGE( "done building peaks" );
     }
-    while ( len );
-
-    /* reopen for reading */
-    /* fflush( fp ); */
-    /* fsync( fileno( fp ) ); */
-    fclose( fp );
-
-    make_peaks_mipmap();
-
-    DMESSAGE( "done building peaks" );
 
     return true;
 }

timeline/src/Engine/Peaks.H

@@ -35,7 +35,9 @@ class Peakfile;
 
 class Peaks
 {
-    mutable volatile bool _pending;
+    /* true if first block is still being built */
+    mutable volatile bool _first_block_pending;
+    mutable volatile bool _mipmaps_pending;
 
     mutable Thread _make_peaks_thread;
     mutable Thread _make_peaks_mipmap_thread;
@@ -110,6 +112,8 @@ class Peaks
 
     mutable float _fpp;
 
+    volatile mutable bool _rescan_needed;
+
     nframes_t read_peaks ( nframes_t s, nframes_t npeaks, nframes_t chunksize ) const;
     nframes_t read_source_peaks ( Peak *peaks, nframes_t s, nframes_t npeaks, nframes_t chunksize ) const;
     nframes_t read_source_peaks ( Peak *peaks, nframes_t npeaks, nframes_t chunksize ) const;
@@ -145,11 +149,11 @@ public:
     bool ready ( nframes_t s, nframes_t npeaks, nframes_t chunksize ) const;
 
     bool make_peaks ( void ) const;
-    bool make_peaks_mipmap ( void ) const;    
     void make_peaks_asynchronously ( void(*callback)(void*), void *userdata ) const;
 
     void prepare_for_writing ( void );
     void finish_writing ( void );
     void write ( sample_t *buf, nframes_t nframes );
 
+    bool needs_more_peaks ( void ) const;
 };

timeline/src/Engine/Timeline.C

@@ -34,11 +34,16 @@
 bool
 Timeline::record ( void )
 {
+    THREAD_ASSERT( UI );
+
+    DMESSAGE( "Initiating recording." );
+
     /* FIXME: right place for this? */
 
     if ( transport->automatically_create_takes() &&
          ! _created_new_takes )
     {
+        DMESSAGE( "Creating new takes." );
         add_take_for_armed_tracks();
         _created_new_takes = true;
     }
@@ -53,12 +58,16 @@ Timeline::record ( void )
 
     if ( transport->punch_enabled() )
     {
+        DMESSAGE( "Finding next punch region following frame %lu...", (unsigned long)frame);
+
         const Sequence_Widget *w = punch_cursor_track->next( frame );
         
         if ( w && w->start() >= frame )
         {
             frame = w->start();
             _punch_out_frame = w->start() + w->length();
+
+            DMESSAGE( "Punch enabled... Will punch in at frame %lu.", (unsigned long)frame );
         }
     }
 
@@ -94,6 +103,8 @@ Timeline::punch_in ( nframes_t frame )
 void
 Timeline::punch_out ( nframes_t frame )
 {
+    THREAD_ASSERT( UI );
+
     for ( int i = tracks->children(); i-- ; )
     {
         Track *t = (Track*)tracks->child( i );
@@ -102,9 +113,11 @@ Timeline::punch_out ( nframes_t frame )
             t->record_ds->stop( frame );
     }
 
-    /* wait until finalization is complete before continuing */
+    DMESSAGE( "Waiting for record threads to shutdown." );
+
+    /* none of the record threads need to call Fl::lock, because we're
+     * holding up the UI thread waiting for them to join.*/
 
-    DMESSAGE( "Waiting for record threads to shutdown" );
     for ( int i = tracks->children(); i-- ; )
     {
         Track *t = (Track*)tracks->child( i );
@@ -113,6 +126,8 @@ Timeline::punch_out ( nframes_t frame )
             t->record_ds->shutdown();
     }
     
+    DMESSAGE( "All record threads stopped." );
+
     _punched_in = false;
     _punch_in_frame = 0;
     _punch_out_frame = 0;
@@ -122,6 +137,8 @@ Timeline::punch_out ( nframes_t frame )
 void
 Timeline::stop ( void )
 {
+    THREAD_ASSERT( UI );
+
     nframes_t frame = transport->frame;
 
     if ( transport->punch_enabled() )

timeline/src/Engine/Track.C

@@ -272,15 +272,19 @@ Track::record ( Capture *c, nframes_t frame )
 
     /* open it again for reading in the GUI thread */
     //   Audio_File *af = Audio_File::from_file( c->audio_file->name() );
+    /* must acquire the FLTK lock because adding a widget might interfere with drawing */
+    Fl::lock();
 
     /* must acquire a write lock because the Audio_Region constructor
-     * will add the region to the specified sequence */
+     * will add the region to the specified sequence, which might affect playback */
     timeline->wrlock();
 
     c->region = new Audio_Region( c->audio_file, sequence(), frame );
 
     timeline->unlock();
 
+    Fl::unlock();
+
     c->region->prepare();
 }
 
@@ -305,14 +309,11 @@ Track::finalize ( Capture *c, nframes_t frame )
     /* adjust region start for latency */
     /* FIXME: is just looking at the first channel good enough? */
 
-    timeline->wrlock();
-
     DMESSAGE( "finalizing audio file" );
-    /* must finalize audio before peaks file, otherwise another thread
-     * might think the peaks are out of date and attempt to regenerate
-     * them */
     c->audio_file->finalize();
 
+    timeline->wrlock();
+
     c->region->finalize( frame );
 
     nframes_t capture_offset = 0;
@@ -333,6 +334,4 @@ Track::finalize ( Capture *c, nframes_t frame )
     c->region->offset( capture_offset );
 
     timeline->unlock();
-
-//    delete c->audio_file;
 }

timeline/src/Project.C

@@ -131,9 +131,7 @@ Project::write_info ( void )
 void
 Project::undo ( void )
 {
-    timeline->wrlock();
     Loggable::undo();
-    timeline->unlock();
 }
 
 bool
@@ -203,12 +201,8 @@ Project::close ( void )
     if ( ! save() )
         return false;
     
-    timeline->wrlock();
-
     Loggable::close();
 
-    timeline->unlock();
-
 //    write_info();
 
     _is_open = false;

timeline/src/Sequence.C

@@ -188,14 +188,11 @@ Sequence::add ( Sequence_Widget *r )
 {
 //    Logger _log( this );
 
-    if ( r->sequence() )
+    if ( r->sequence() && r->sequence() != this )
     {
         /* This method can be called from the Capture thread as well as the GUI thread, so we must lock FLTK before redraw */
-        Fl::lock();
         r->redraw();
         r->sequence()->remove( r );
-        Fl::unlock();
-//        r->track()->redraw();
     }
 
     r->sequence( this );
@@ -481,28 +478,30 @@ Sequence::handle ( int m )
                         Sequence_Widget::pushed( NULL );
                 }
 
-                Loggable::block_start();
+                if ( _delete_queue.size() )
-
-                timeline->wrlock();
-                while ( _delete_queue.size() )
                 {
+                    Loggable::block_start();
                     
-                    Sequence_Widget *t = _delete_queue.front();
+                    while ( _delete_queue.size() )
-                    _delete_queue.pop();
-
-                    if ( Sequence_Widget::pushed() == t )
-                        Sequence_Widget::pushed( NULL );
-                    if ( Sequence_Widget::belowmouse() == t )
                     {
-                        Sequence_Widget::belowmouse()->handle( FL_LEAVE );
+                        Sequence_Widget *t = _delete_queue.front();
-                        Sequence_Widget::belowmouse( NULL );
+                        _delete_queue.pop();
-                    }
                         
-                    delete t;
+                        if ( Sequence_Widget::pushed() == t )
+                            Sequence_Widget::pushed( NULL );
+                        if ( Sequence_Widget::belowmouse() == t )
+                        {
+                            Sequence_Widget::belowmouse()->handle( FL_LEAVE );
+                            Sequence_Widget::belowmouse( NULL );
+                        }
+
+                        timeline->wrlock();
+                        delete t;
+                        timeline->unlock();
                 }
-                timeline->unlock();
 
                 Loggable::block_end();
+                }
 
                 if ( m == FL_PUSH )
                     return 1;
@@ -550,6 +549,7 @@ const Sequence_Widget *
         Sequence::next ( nframes_t from ) const
     {
         for ( list <Sequence_Widget*>::const_iterator i = _widgets.begin(); i != _widgets.end(); i++ )
+//            if ( (*i)->start() >= from )
             if ( (*i)->start() > from )
                 return *i;
 

timeline/src/TLE.fl

@@ -396,7 +396,7 @@ Project::compact();}
           } {
             MenuItem {} {
               label Undo
-              callback {Project::undo();}
+              callback {timeline->command_undo();}
               xywh {5 5 40 25} shortcut 0x4007a divider
             }
             MenuItem {} {

timeline/src/Timeline.C

@@ -1253,6 +1253,8 @@ Timeline::draw_cursors ( void ) const
 void
 Timeline::draw ( void )
 {
+    /* Any code that might affect the structures used for drawing from
+     * another thread must use Fl::lock()/unlock()! */ 
     THREAD_ASSERT( UI );
 
     int X, Y, W, H;
@@ -1260,8 +1262,6 @@ Timeline::draw ( void )
     int bdx = 0;
     int bdw = 0;
 
-    rdlock();
-
     X = tracks->x() + bdx + 1;
     Y = tracks->y();
     W = tracks->w() - bdw - 1;
@@ -1363,8 +1363,6 @@ done:
 
     _old_xposition = xoffset;
     _old_yposition = panzoomer->y_value();
-
-    unlock();
 }
 
 void
@@ -1438,6 +1436,7 @@ Timeline::redraw_playhead ( void )
             /* we've passed one or more punch regions... punch in for the next, if available. */
             const Sequence_Widget *w = punch_cursor_track->next( transport->frame );
             
+            DMESSAGE( "Delayed punch in" );
             if ( w && 
                  w->start() > transport->frame )
             {
@@ -1993,15 +1992,27 @@ Timeline::command_remove_track ( Track *track )
 }
 
 void
-Timeline::command_quit ( )
+Timeline::command_quit ( void )
 {
+    timeline->wrlock();
+
     Project::close();
 
+    timeline->unlock();
+  
     command_save();
   
     while ( Fl::first_window() ) Fl::first_window()->hide();
 }
 
+void
+Timeline::command_undo ( void )
+{
+    wrlock();
+    Project::undo();
+    unlock();
+}
+
 bool
 Timeline::command_load ( const char *name, const char *display_name )
 {

timeline/src/Timeline.H

@@ -257,6 +257,7 @@ public:
 
     void command_move_track_up ( Track *track );
     void command_move_track_down ( Track *track );
+    void command_undo ( void );
 
     int find_track ( const Track * track ) const;