From 9667f98995732872df5e29047cd6a828621b4e84 Mon Sep 17 00:00:00 2001 From: Jonathan Moore Liles Date: Sun, 1 Apr 2012 18:27:27 -0700 Subject: [PATCH] Timeline: Don't fork to generate peak mipmaps, do it in the capture thread. Also, clean up related concurrency issues. --- timeline/src/Audio_Region.C | 53 ++------- timeline/src/Audio_Region.H | 3 - timeline/src/Engine/Audio_File.C | 17 +-- timeline/src/Engine/Audio_File.H | 6 +- timeline/src/Engine/Audio_File_SF.C | 14 ++- timeline/src/Engine/Audio_Region.C | 25 +--- timeline/src/Engine/Peaks.C | 175 +++++++++++++++++++++++----- timeline/src/Engine/Peaks.H | 12 +- timeline/src/Engine/Track.C | 13 ++- 9 files changed, 205 insertions(+), 113 deletions(-) diff --git a/timeline/src/Audio_Region.C b/timeline/src/Audio_Region.C index 34e4f53..9e8ac33 100644 --- a/timeline/src/Audio_Region.C +++ b/timeline/src/Audio_Region.C @@ -405,44 +405,6 @@ Audio_Region::draw_fade ( const Fade &fade, Fade::fade_dir_e dir, bool line, int fl_pop_matrix(); } -struct Peaks_Redraw_Request { - - Audio_Region *region; - - nframes_t start; - nframes_t end; - - Peaks_Redraw_Request ( Audio_Region *region, nframes_t start, nframes_t end ) : region( region ), start( start), end( end ) - { - } -}; - -/* static wrapper */ -void -Audio_Region::peaks_pending_cb ( void *v ) -{ - Peaks_Redraw_Request *r = (Peaks_Redraw_Request*)v; - - r->region->peaks_pending_cb( r ); -} - -void -Audio_Region::peaks_pending_cb ( Peaks_Redraw_Request *r ) -{ - int npeaks = timeline->ts_to_x( r->end - r->start ); - - if ( _clip->peaks()->ready( r->start, npeaks, timeline->fpp() ) ) - { - printf( "damaging from timeout\n" ); - /* FIXME: only need to damage the affected area! */ - timeline->damage( FL_DAMAGE_ALL, x(), y(), w(), h() ); - - delete r; - } - else - Fl::repeat_timeout( 0.5f, &Audio_Region::peaks_pending_cb, (void*)r ); -} - void Audio_Region::draw_box( void ) { @@ -503,6 +465,8 @@ Audio_Region::draw ( void ) X -= 2; W += 4; + Fl_Color c = selected() ? fl_invert_color( _color ) : _color; + /* start with region length... */ // int rw = timeline->ts_to_x( min( length(), timeline->x_to_ts( sequence()->w() ) ) ); int rw = W; @@ -537,6 +501,7 @@ Audio_Region::draw ( void ) if ( X - rx > 0 ) offset += timeline->x_to_ts( X - rx ); +// DMESSAGE( "Drawing audio region."); do { @@ -548,6 +513,12 @@ Audio_Region::draw ( void ) int loop_peaks_needed = _loop ? timeline->ts_to_x( _loop ) : timeline->ts_to_x( _clip->length() ); + if ( this == ((Audio_Sequence*)sequence())->capture_region() ) + { + loop_peaks_needed = timeline->ts_to_x( _range.length ); + c = FL_BLACK; + } + if ( ! loop_peaks_needed ) break; @@ -627,15 +598,13 @@ Audio_Region::draw ( void ) loop_peaks_needed, ch, pbuf + i, peaks, channels, - selected() ? fl_invert_color( _color ) : _color ); + c ); } } if ( peaks < loop_peaks_needed ) { - /* couldn't read peaks--perhaps they're being generated. Try again later. */ - Fl::add_timeout( 0.5f, &Audio_Region::peaks_pending_cb, - new Peaks_Redraw_Request( this, start + timeline->x_to_ts( peaks ), end ) ); +// DMESSAGE( "Peak read came up %lu peaks short", (unsigned long) loop_peaks_needed - peaks ); } xo += loop_peaks_needed; diff --git a/timeline/src/Audio_Region.H b/timeline/src/Audio_Region.H index 881f76d..44619df 100644 --- a/timeline/src/Audio_Region.H +++ b/timeline/src/Audio_Region.H @@ -23,7 +23,6 @@ #include "Sequence_Region.H" class Audio_File; -class Peaks_Redraw_Request; class Fl_Menu_; class Fl_Menu_Button; @@ -105,8 +104,6 @@ private: friend class Track; /* for _clip */ Fl_Menu_Button & menu ( void ); - static void peaks_pending_cb ( void *v ); - void peaks_pending_cb ( Peaks_Redraw_Request *r ); static void menu_cb ( Fl_Widget *w, void *v ); void menu_cb ( const Fl_Menu_ *m ); diff --git a/timeline/src/Engine/Audio_File.C b/timeline/src/Engine/Audio_File.C index 82dab02..ffe2b00 100644 --- a/timeline/src/Engine/Audio_File.C +++ b/timeline/src/Engine/Audio_File.C @@ -37,6 +37,9 @@ Audio_File::~Audio_File ( ) if ( _filename ) free( _filename ); + + if ( _path ) + free( _path ); } const Audio_File::format_desc * @@ -66,23 +69,23 @@ is_absolute ( const char *name ) return *name == '/'; } -/** return a static pointer to /name/ corrected for relative path. */ -const char *Audio_File::realname ( const char *name ) +/** return pointer to /name/ corrected for relative path. */ +char *Audio_File::path ( const char *name ) { - static char rname[512]; + char *path = 0; if ( is_absolute( name ) ) - strncpy( rname, name, sizeof( rname ) ); + path = strdup( name ); else - snprintf( rname, sizeof( rname ), "sources/%s", name ); + asprintf( &path, "sources/%s", name ); - return rname; + return path; } const char * Audio_File::filename ( void ) const { - return realname( _filename ); + return _path; } /** attempt to open any supported filetype */ diff --git a/timeline/src/Engine/Audio_File.H b/timeline/src/Engine/Audio_File.H index 0242d15..29a2f88 100644 --- a/timeline/src/Engine/Audio_File.H +++ b/timeline/src/Engine/Audio_File.H @@ -52,6 +52,8 @@ protected: }; char *_filename; + char *_path; + volatile nframes_t _length; /* length of file in samples */ nframes_t _samplerate; /* sample rate */ int _channels; @@ -60,13 +62,13 @@ protected: static const format_desc * find_format ( const format_desc *fd, const char *name ); - static const char *realname ( const char *name ); + static char *path ( const char *name ); public: Audio_File ( ) : _peaks( this ) { - _filename = NULL; + _path =_filename = NULL; _samplerate = 0; _length = _channels = 0; _refs = 0; diff --git a/timeline/src/Engine/Audio_File_SF.C b/timeline/src/Engine/Audio_File_SF.C index c381411..d7f363a 100644 --- a/timeline/src/Engine/Audio_File_SF.C +++ b/timeline/src/Engine/Audio_File_SF.C @@ -33,6 +33,7 @@ #include "const.h" #include "debug.h" +#include @@ -62,7 +63,9 @@ Audio_File_SF::from_file ( const char *filename ) memset( &si, 0, sizeof( si ) ); - if ( ! ( in = sf_open( realname( filename ), SFM_READ, &si ) ) ) + char *fp = path( filename ); + + if ( ! ( in = sf_open( fp, SFM_READ, &si ) ) ) return NULL; /* if ( si.samplerate != timeline->sample_rate() ) */ @@ -76,6 +79,7 @@ Audio_File_SF::from_file ( const char *filename ) // c->_peak_writer = NULL; c->_current_read = 0; c->_filename = strdup( filename ); + c->_path = fp; c->_length = si.frames; c->_samplerate = si.samplerate; c->_channels = si.channels; @@ -99,6 +103,7 @@ Audio_File_SF::create ( const char *filename, nframes_t samplerate, int channels memset( &si, 0, sizeof( si ) ); + const Audio_File::format_desc *fd = Audio_File::find_format( Audio_File_SF::supported_formats, format ); if ( ! fd ) @@ -111,7 +116,9 @@ Audio_File_SF::create ( const char *filename, nframes_t samplerate, int channels char *name; asprintf( &name, "%s.%s", filename, fd->extension ); - if ( ! ( out = sf_open( realname( name ), SFM_WRITE, &si ) ) ) + char *filepath = path( name ); + + if ( ! ( out = sf_open( filepath, SFM_WRITE, &si ) ) ) { printf( "couldn't create soundfile.\n" ); free( name ); @@ -120,6 +127,7 @@ Audio_File_SF::create ( const char *filename, nframes_t samplerate, int channels Audio_File_SF *c = new Audio_File_SF; + c->_path = filepath; c->_filename = name; c->_length = 0; c->_samplerate = samplerate; @@ -141,7 +149,7 @@ Audio_File_SF::open ( void ) memset( &si, 0, sizeof( si ) ); - if ( ! ( _in = sf_open( realname( _filename ), SFM_READ, &si ) ) ) + if ( ! ( _in = sf_open( _path, SFM_READ, &si ) ) ) return false; _current_read = 0; diff --git a/timeline/src/Engine/Audio_Region.C b/timeline/src/Engine/Audio_Region.C index 35ca784..465c1fc 100644 --- a/timeline/src/Engine/Audio_Region.C +++ b/timeline/src/Engine/Audio_Region.C @@ -208,34 +208,22 @@ Audio_Region::write ( nframes_t nframes ) { THREAD_ASSERT( Capture ); - _range.length += nframes; - - /* FIXME: too much? */ -// _track->damage( FL_DAMAGE_EXPOSE, x() + w(), y(), 10/* FIXME: guess */, h() ); - if ( 0 == ( timeline->ts_to_x( _range.length ) % 20 ) ) { - nframes_t oldl = _clip->length(); - - /* get the new size. Remember, this is a read-only handle on the source--not the same - one being written to */ - _clip->close(); - _clip->open(); - - int W = timeline->ts_to_x( _clip->length() - oldl ); - - /* why - 1? */ + int W = 20; if ( W ) { - ++W; Fl::lock(); - sequence()->damage( FL_DAMAGE_ALL, x() + w() - W, y(), W, h() ); -// Fl::awake(); +// sequence()->damage( FL_DAMAGE_ALL, ( x() + w() - W ) - 20, y(), W, h() ); + sequence()->damage( FL_DAMAGE_ALL, x(), y(), w(), h() ); + // Fl::awake(); Fl::unlock(); } } + _range.length += nframes; + return nframes; } @@ -255,7 +243,6 @@ Audio_Region::finalize ( nframes_t frame ) _clip->close(); _clip->open(); - /* FIXME: should we attempt to truncate the file? */ Fl::lock(); redraw(); Fl::awake(); diff --git a/timeline/src/Engine/Peaks.C b/timeline/src/Engine/Peaks.C index a587f70..443af38 100644 --- a/timeline/src/Engine/Peaks.C +++ b/timeline/src/Engine/Peaks.C @@ -24,6 +24,8 @@ /* Code for peakfile reading, resampling, generation and streaming */ +#include + #include #include #include @@ -68,20 +70,21 @@ Peaks::peakbuffer Peaks::_peakbuf; static -const char * +char * peakname ( const char *filename ) { - static char file[512]; + char *file; - snprintf( file, 512, "%s.peak", filename ); + asprintf( &file, "%s.peak", filename ); - return (const char*)&file; + return file; } Peaks::Peaks ( Audio_File *c ) { + _pending = false; _clip = c; _peak_writer = NULL; } @@ -176,7 +179,7 @@ public: // printf( "chunksize=%lu, skip=%lu\n", (unsigned long)bh.chunksize, (unsigned long) bh.skip ); - ASSERT( bh.chunksize, "Invalid peak file structure!" ); + ASSERT( bh.chunksize, "Chucksize of zero. Invalid peak file structure!" ); blocks.push_back( block_descriptor( bh.chunksize, ftell( _fp ) ) ); @@ -192,7 +195,7 @@ public: } if ( ! blocks.size() ) - FATAL( "invalid peak file?" ); + FATAL( "Peak file contains no blocks!" ); // DMESSAGE( "peakfile has %d blocks.", blocks.size() ); @@ -251,8 +254,16 @@ public: _chunksize = 0; _channels = channels; - if ( ! ( _fp = fopen( peakname( name ), "r" ) ) ) + char *pn = peakname( name ); + + if ( ! ( _fp = fopen( pn, "r" ) ) ) + { + WARNING( "Failed to open peakfile for reading: %s", strerror(errno) ); + free( pn ); return false; + } + + free( pn ); scan( chunksize ); @@ -297,14 +308,19 @@ public: read_peaks ( Peak *peaks, nframes_t s, int npeaks, nframes_t chunksize ) { if ( ! _fp ) + { + DMESSAGE( "No peakfile open, WTF?" ); return 0; + } const unsigned int ratio = chunksize / _chunksize; /* locate to start position */ if ( fseek( _fp, _offset + ( frame_to_peak( s ) * sizeof( Peak ) ), SEEK_SET ) ) - /* failed to seek... peaks not ready? */ + { + DMESSAGE( "failed to seek... peaks not ready?" ); return 0; + } if ( ratio == 1 ) return fread( peaks, sizeof( Peak ) * _channels, npeaks, _fp ); @@ -355,6 +371,9 @@ public: bool Peaks::ready ( nframes_t s, int npeaks, nframes_t chunksize ) const { + /* if ( _pending ) */ + /* return false; */ + Peakfile _peakfile; if ( ! _peakfile.open( _clip->filename(), _clip->channels(), chunksize ) ) @@ -369,20 +388,25 @@ Peaks::read_peakfile_peaks ( Peak *peaks, nframes_t s, int npeaks, nframes_t chu /* never try to build peaks while recording */ if ( ! transport->recording ) { - if ( ! current() ) + if ( ! current() && ! _pending ) { /* Build peaks asyncronously */ - if ( ! fork() ) - exit( make_peaks() ); - else - return 0; + _pending = true; + _make_peaks_thread.clone( &Peaks::make_peaks, const_cast(this) ); + _make_peaks_thread.detach(); } } + /* if ( _pending ) */ + /* return 0; */ + Peakfile _peakfile; if ( ! _peakfile.open( _clip->filename(), _clip->channels(), chunksize ) ) + { + DMESSAGE( "Failed to open peakfile!" ); return 0; + } return _peakfile.read_peaks( peaks, s, npeaks, chunksize ); } @@ -458,9 +482,13 @@ Peaks::read_peaks ( nframes_t s, int npeaks, nframes_t chunksize ) const /* FIXME: use actual minimum chunksize from peakfile! */ if ( chunksize < (nframes_t)cache_minimum ) + { _peakbuf.len = read_source_peaks( _peakbuf.buf->data, s, npeaks, chunksize ); + } else + { _peakbuf.len = read_peakfile_peaks( _peakbuf.buf->data, s, npeaks, chunksize ); + } return _peakbuf.len; } @@ -469,7 +497,22 @@ Peaks::read_peaks ( nframes_t s, int npeaks, nframes_t chunksize ) const bool Peaks::current ( void ) const { - return ! newer( _clip->filename(), peakname( _clip->filename() ) ); + char *pn = peakname( _clip->filename() ); + + bool b = ! newer( _clip->filename(), pn ); + + free( pn ); + + return b; +} + +/* thread entry point */ +void * +Peaks::make_peaks ( void *v ) +{ + ((Peaks*)v)->make_peaks(); + + return NULL; } bool @@ -477,7 +520,24 @@ Peaks::make_peaks ( void ) const { Peaks::Builder pb( this ); - return pb.make_peaks(); + int b = pb.make_peaks(); + + _pending = false; + + Fl::lock(); + timeline->redraw(); + Fl::unlock(); + + return b; +} + +/* thread entry point */ +void * +Peaks::make_peaks_mipmap ( void *v ) +{ + ((Peaks*)v)->make_peaks_mipmap(); + + return NULL; } bool @@ -485,7 +545,11 @@ Peaks::make_peaks_mipmap ( void ) const { Peaks::Builder pb( this ); - return pb.make_peaks_mipmap(); + bool b = pb.make_peaks_mipmap(); + + _pending = false; + + return b; } /** return normalization factor for a single peak, assuming the peak @@ -511,7 +575,11 @@ Peaks::prepare_for_writing ( void ) assert( ! _peak_writer ); - _peak_writer = new Peaks::Streamer( _clip->filename(), _clip->channels(), cache_minimum ); + char *pn = peakname( _clip->filename() ); + + _peak_writer = new Peaks::Streamer( pn, _clip->channels(), cache_minimum ); + + free( pn ); } void @@ -522,10 +590,7 @@ Peaks::finish_writing ( void ) delete _peak_writer; _peak_writer = NULL; -/* now fill in the rest of the cache */ - if ( ! fork() ) - exit( make_peaks_mipmap() ); - + make_peaks_mipmap(); } void @@ -556,9 +621,9 @@ Peaks::Streamer::Streamer ( const char *filename, int channels, nframes_t chunks _peak = new Peak[ channels ]; memset( _peak, 0, sizeof( Peak ) * channels ); - if ( ! ( _fp = fopen( peakname( filename ), "w" ) ) ) + if ( ! ( _fp = fopen( filename, "w" ) ) ) { - WARNING( "could not open peakfile for streaming." ); + FATAL( "could not open peakfile for streaming." ); } peakfile_block_header bh; @@ -575,8 +640,12 @@ Peaks::Streamer::~Streamer ( ) { /* fwrite( _peak, sizeof( Peak ) * _channels, 1, _fp ); */ + fflush( _fp ); + touch( fileno( _fp ) ); +// fsync( fileno( _fp ) ); + fclose( _fp ); delete[] _peak; @@ -595,7 +664,7 @@ Peaks::Streamer::write ( const sample_t *buf, nframes_t nframes ) fwrite( _peak, sizeof( Peak ) * _channels, 1, _fp ); /* FIXME: shouldn't we just use write() instead? */ - fflush( _fp ); +// fflush( _fp ); memset( _peak, 0, sizeof( Peak ) * _channels ); @@ -648,13 +717,13 @@ Peaks::Builder::write_block_header ( nframes_t chunksize ) fseek( fp, last_block_pos - sizeof( peakfile_block_header ), SEEK_SET ); // fseek( fp, 0 - sizeof( bh ), SEEK_CUR ); -// DMESSAGE( "old block header: chunksize=%lu, skip=%lu", bh.chunksize, bh.skip ); +// DMESSAGE( "old block header: chunksize=%lu, skip=%lu", (unsigned long) bh.chunksize, (unsigned long) bh.skip ); bh.skip = pos - last_block_pos; ASSERT( bh.skip, "Attempt to create empty block. pos=%lu, last_block_pos=%lu", pos, last_block_pos ); -// DMESSAGE( "new block header: chunksize=%lu, skip=%lu", bh.chunksize, bh.skip ); +// DMESSAGE( "new block header: chunksize=%lu, skip=%lu", (unsigned long) bh.chunksize, (unsigned long) bh.skip ); fwrite( &bh, sizeof( bh ), 1, fp ); @@ -673,6 +742,9 @@ 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 ) @@ -683,22 +755,47 @@ Peaks::Builder::make_peaks_mipmap ( void ) Audio_File *_clip = _peaks->_clip; const char *filename = _clip->filename(); + char *pn = peakname( filename ); FILE *rfp; + + if ( ! ( rfp = fopen( pn, "r" ) ) ) + { + WARNING( "could not open peakfile for reading: %s.", strerror( errno ) ); + free( pn ); + return false; + } - rfp = fopen( peakname( filename ), "r" ); + { + peakfile_block_header bh; + + fread( &bh, sizeof( peakfile_block_header ), 1, rfp ); + + if ( bh.skip ) + { + WARNING( "Peakfile already has multiple blocks..." ); + fclose( rfp ); + free( pn ); + return false; + } + + } last_block_pos = sizeof( peakfile_block_header ); /* open for reading */ // rfp = fopen( peakname( filename ), "r" ); + /* open the file again for appending */ - if ( ! ( fp = fopen( peakname( filename ), "r+" ) ) ) + if ( ! ( fp = fopen( pn, "r+" ) ) ) { - WARNING( "could not open peakfile for appending." ); + WARNING( "could not open peakfile for appending: %s.", strerror( errno ) ); + free( pn ); return false; } + free( pn ); + if ( fseek( fp, 0, SEEK_END ) ) FATAL( "error performing seek: %s", strerror( errno ) ); @@ -714,9 +811,10 @@ Peaks::Builder::make_peaks_mipmap ( void ) * preceding level */ nframes_t cs = Peaks::cache_minimum << Peaks::cache_step; + for ( int i = 1; i < Peaks::cache_levels; ++i, cs <<= Peaks::cache_step ) { - DMESSAGE( "building level %d peak cache", i + 1 ); + DMESSAGE( "building level %d peak cache cs=%i", i + 1, cs ); /* DMESSAGE( "%lu", _clip->length() / cs ); */ @@ -726,10 +824,10 @@ Peaks::Builder::make_peaks_mipmap ( void ) break; } - Peakfile pf; /* open the peakfile for the previous cache level */ + pf.open( rfp, _clip->channels(), cs >> Peaks::cache_step ); // pf.open( _clip->filename(), _clip->channels(), cs >> Peaks::cache_step ); @@ -740,12 +838,16 @@ Peaks::Builder::make_peaks_mipmap ( void ) nframes_t s = 0; do { len = pf.read_peaks( buf, s, 1, cs ); + s += cs; fwrite( buf, sizeof( buf ), len, fp ); } while ( len ); + /* fflush( fp ); */ + /* fsync( fileno( fp ) ); */ + pf.leave_open(); } @@ -766,8 +868,15 @@ Peaks::Builder::make_peaks ( void ) DMESSAGE( "building peaks for \"%s\"", filename ); - if ( ! ( fp = fopen( peakname( filename ), "w+" ) ) ) + char *pn = peakname( filename ); + + if ( ! ( fp = fopen( pn, "w+" ) ) ) + { + free( pn ); return false; + } + + free( pn ); _clip->seek( 0 ); @@ -787,6 +896,8 @@ Peaks::Builder::make_peaks ( void ) while ( len ); /* reopen for reading */ + /* fflush( fp ); */ + /* fsync( fileno( fp ) ); */ fclose( fp ); make_peaks_mipmap(); diff --git a/timeline/src/Engine/Peaks.H b/timeline/src/Engine/Peaks.H index 423a310..a719902 100644 --- a/timeline/src/Engine/Peaks.H +++ b/timeline/src/Engine/Peaks.H @@ -27,10 +27,20 @@ #include +#include "Thread.H" + + class Audio_File; class Peaks { + mutable volatile bool _pending; + + mutable Thread _make_peaks_thread; + mutable Thread _make_peaks_mipmap_thread; + + static void * make_peaks_mipmap ( void *v ); + static void * make_peaks ( void *v ); struct peakdata { @@ -115,8 +125,6 @@ public: static const int cache_levels; static const int cache_step; - - Peaks ( Audio_File *c ); ~Peaks ( ); diff --git a/timeline/src/Engine/Track.C b/timeline/src/Engine/Track.C index 6e2ad80..cf7c3a5 100644 --- a/timeline/src/Engine/Track.C +++ b/timeline/src/Engine/Track.C @@ -249,12 +249,14 @@ Track::record ( Capture *c, nframes_t frame ) { THREAD_ASSERT( Capture ); - char pat[256]; + char *pat; - snprintf( pat, sizeof( pat ), "%s-%llu", name(), uuid() ); + asprintf( &pat, "%s-%llu", name(), uuid() ); c->audio_file = Audio_File_SF::create( pat, engine->sample_rate(), input.size(), Track::capture_format ); + free( pat ); + if ( ! c->audio_file ) FATAL( "Could not create file for new capture!" ); @@ -287,10 +289,15 @@ Track::finalize ( Capture *c, nframes_t frame ) /* adjust region start for latency */ /* FIXME: is just looking at the first channel good enough? */ - c->region->finalize( frame ); 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(); + /* peaks get finalized here */ + c->region->finalize( frame ); + nframes_t capture_offset = 0; /* Add the system latency twice. Once for the input (usually