fixed peak waveform issue introduced by the ftruncate preallocation of peakfile....

git-svn-id: svn://localhost/ardour2/trunk@1250 d708f5d6-7413-0410-9779-e7cbd77b26cf

diff --git a/libs/ardour/ardour/audiosource.h b/libs/ardour/ardour/audiosource.h
index 748264e7cb39992a776ef1a97b10a45dbb923c39..251f19f2e55c56eaf4307a474fa040fd886bd1f9 100644 (file)
--- a/libs/ardour/ardour/audiosource.h
+++ b/libs/ardour/ardour/audiosource.h
@@ -127,6 +127,9 @@ const nframes_t frames_per_peak = 256;
        void build_peaks_from_scratch ();
 
        int  do_build_peak (nframes_t, nframes_t);
+       void truncate_peakfile();
+
+       mutable off_t _peak_byte_max; // modified in do_build_peaks()
 
        virtual nframes_t read_unlocked (Sample *dst, nframes_t start, nframes_t cnt) const = 0;
        virtual nframes_t write_unlocked (Sample *dst, nframes_t cnt) = 0;
@@ -151,7 +154,7 @@ const nframes_t frames_per_peak = 256;
        static vector<boost::shared_ptr<AudioSource> > pending_peak_sources;
        static Glib::Mutex* pending_peak_sources_lock;
 
-       static void queue_for_peaks (boost::shared_ptr<AudioSource>);
+       static void queue_for_peaks (boost::shared_ptr<AudioSource>, bool notify=true);
        static void clear_queue_for_peaks ();
        
        struct PeakBuildRecord {

diff --git a/libs/ardour/audiofilesource.cc b/libs/ardour/audiofilesource.cc
index 6999151a1c07bab2848ab44461772e5b07cf1346..92db950cc0ddb9f2713f5eed1e71d5d995848a4c 100644 (file)
--- a/libs/ardour/audiofilesource.cc
+++ b/libs/ardour/audiofilesource.cc
@@ -22,6 +22,7 @@
 #include <sys/time.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include <fcntl.h>
 #include <errno.h>
 
 #include <pbd/mountpoint.h>
@@ -257,7 +258,7 @@ AudioFileSource::mark_streaming_write_completed ()
 
        if (_peaks_built || pending_peak_builds.empty()) {
                _peaks_built = true;
-                PeaksReady (); /* EMIT SIGNAL */
+               PeaksReady (); /* EMIT SIGNAL */
        }
 }
 

diff --git a/libs/ardour/audiosource.cc b/libs/ardour/audiosource.cc
index cc6d156c9c703e1bfbc7364ac80e136ea5b420d4..bfe180240bfcb59e14b7da50f6f1997c5486a941 100644 (file)
--- a/libs/ardour/audiosource.cc
+++ b/libs/ardour/audiosource.cc
@@ -60,6 +60,7 @@ AudioSource::AudioSource (Session& s, string name)
        }
 
        _peaks_built = false;
+       _peak_byte_max = 0;
        next_peak_clear_should_notify = true;
        _read_data_count = 0;
        _write_data_count = 0;
@@ -73,6 +74,7 @@ AudioSource::AudioSource (Session& s, const XMLNode& node)
        }
 
        _peaks_built = false;
+       _peak_byte_max = 0;
        next_peak_clear_should_notify = true;
        _read_data_count = 0;
        _write_data_count = 0;
@@ -252,13 +254,13 @@ AudioSource::stop_peak_thread ()
 }
 
 void 
-AudioSource::queue_for_peaks (boost::shared_ptr<AudioSource> source)
+AudioSource::queue_for_peaks (boost::shared_ptr<AudioSource> source, bool notify)
 {
        if (have_peak_thread) {
                
                Glib::Mutex::Lock lm (*pending_peak_sources_lock);
                
-               source->next_peak_clear_should_notify = true;
+               source->next_peak_clear_should_notify = notify;
                
                if (find (pending_peak_sources.begin(),
                          pending_peak_sources.end(),
@@ -385,8 +387,10 @@ AudioSource::initialize_peakfile (bool newfile, string audio_path)
                                
                                if (!err && stat_file.st_mtime > statbuf.st_mtime){
                                        _peaks_built = false;
+                                       _peak_byte_max = 0;
                                } else {
                                        _peaks_built = true;
+                                       _peak_byte_max = statbuf.st_size;
                                }
                        }
                }
@@ -488,7 +492,7 @@ AudioSource::read_peaks (PeakData *peaks, nframes_t npeaks, nframes_t start, nfr
 
                /* open, read, close */
 
-               if ((peakfile = ::open (peakpath.c_str(), O_RDWR|O_CREAT, 0664)) < 0) {
+               if ((peakfile = ::open (peakpath.c_str(), O_RDONLY, 0664)) < 0) {
                        error << string_compose(_("AudioSource: cannot open peakpath \"%1\" (%2)"), peakpath, strerror (errno)) << endmsg;
                        return -1;
                }
@@ -562,7 +566,7 @@ AudioSource::read_peaks (PeakData *peaks, nframes_t npeaks, nframes_t start, nfr
 
                /* open ... close during out: handling */
 
-               if ((peakfile = ::open (peakpath.c_str(), O_RDWR|O_CREAT, 0664)) < 0) {
+               if ((peakfile = ::open (peakpath.c_str(), O_RDONLY, 0664)) < 0) {
                        error << string_compose(_("AudioSource: cannot open peakpath \"%1\" (%2)"), peakpath, strerror (errno)) << endmsg;
                        return 0;
                }
@@ -773,6 +777,7 @@ AudioSource::build_peaks ()
                }
 
                if (pr_signal) {
+                       truncate_peakfile();
                        PeaksReady (); /* EMIT SIGNAL */
                }
        }
@@ -845,25 +850,32 @@ AudioSource::do_build_peak (nframes_t first_frame, nframes_t cnt)
                cnt -= frames_read;
        }
 
-#define BLOCKSIZE (256 * 1024)
-
-       target_length = BLOCKSIZE * ((first_peak_byte + BLOCKSIZE + 1) / BLOCKSIZE);
+#define BLOCKSIZE (128 * 1024)
 
        /* on some filesystems (ext3, at least) this helps to reduce fragmentation of
           the peakfiles. its not guaranteed to do so, and even on ext3 (as of december 2006)
           it does not cause single-extent allocation even for peakfiles of 
-          less than BLOCKSIZE bytes.
+          less than BLOCKSIZE bytes.  only call ftruncate if we'll make the file larger.
        */
+       off_t endpos = lseek (peakfile, 0, SEEK_END);
+               
+       target_length = BLOCKSIZE * ((first_peak_byte + BLOCKSIZE + 1) / BLOCKSIZE);
 
-       ftruncate (peakfile, target_length);
+       if (endpos < target_length) {
+               // XXX - we really shouldn't be doing this for destructive source peaks
+               ftruncate (peakfile, target_length);
+               //cerr << "do build TRUNC: " << peakpath << "  " << target_length << endl;
 
-       /* error doesn't actually matter though, so continue on without testing */
+               /* error doesn't actually matter though, so continue on without testing */
+       }
 
        if (::pwrite (peakfile, peakbuf, sizeof (PeakData) * peaki, first_peak_byte) != (ssize_t) (sizeof (PeakData) * peaki)) {
                error << string_compose(_("%1: could not write peak file data (%2)"), _name, strerror (errno)) << endmsg;
                goto out;
        }
 
+       _peak_byte_max = max(_peak_byte_max, first_peak_byte + sizeof(PeakData)*peaki);
+
        ret = 0;
 
   out:
@@ -881,7 +893,28 @@ AudioSource::build_peaks_from_scratch ()
 
        next_peak_clear_should_notify = true;
        pending_peak_builds.push_back (new PeakBuildRecord (0, _length));
-       queue_for_peaks (shared_from_this());
+       queue_for_peaks (shared_from_this(), true);
+}
+
+void
+AudioSource::truncate_peakfile ()
+{
+       int peakfile = -1;
+
+       /* truncate the peakfile down to its natural length if necessary */
+
+       if ((peakfile = ::open (peakpath.c_str(), O_RDWR)) >= 0) {
+               off_t end = lseek (peakfile, 0, SEEK_END);
+               
+               if (end > _peak_byte_max) {
+                       ftruncate(peakfile, _peak_byte_max);
+                       //cerr << "truncated " << peakpath << " to " << _peak_byte_max << " bytes" << endl;
+               }
+               else {
+                       //cerr << "NOT truncated " << peakpath << " to " << _peak_byte_max << " end " << end << endl;
+               }
+               close (peakfile);
+       }
 }
 
 bool
@@ -903,26 +936,19 @@ AudioSource::file_changed (string path)
 nframes_t
 AudioSource::available_peaks (double zoom_factor) const
 {
-       int peakfile;
        off_t end;
 
        if (zoom_factor < frames_per_peak) {
                return length(); // peak data will come from the audio file
        } 
        
-       /* peak data comes from peakfile */
-
-       if ((peakfile = ::open (peakpath.c_str(), O_RDONLY)) < 0) {
-               error << string_compose(_("AudioSource: cannot open peakpath \"%1\" (%2)"), peakpath, strerror (errno)) << endmsg;
-               return 0;
-       }
-
-       { 
-               Glib::Mutex::Lock lm (_lock);
-               end = lseek (peakfile, 0, SEEK_END);
-       }
+       /* peak data comes from peakfile, but the filesize might not represent
+          the valid data due to ftruncate optimizations, so use _peak_byte_max state.
+          XXX - there might be some atomicity issues here, we should probably add a lock,
+          but _peak_byte_max only monotonically increases after initialization.
+       */
 
-       close (peakfile);
+       end = _peak_byte_max;
 
        return (end/sizeof(PeakData)) * frames_per_peak;
 }

diff --git a/libs/ardour/sndfilesource.cc b/libs/ardour/sndfilesource.cc
index 91605049caa4c381554d207bea3dee503220a028..fa2f432ee6b507f09d9d75ceb1d8dd8fa2de094e 100644 (file)
--- a/libs/ardour/sndfilesource.cc
+++ b/libs/ardour/sndfilesource.cc
@@ -408,26 +408,25 @@ SndFileSource::nondestructive_write_unlocked (Sample *data, nframes_t cnt)
                PeakBuildRecord *pbr = 0;
                
                if (pending_peak_builds.size()) {
-                               pbr = pending_peak_builds.back();
-                       }
+                       pbr = pending_peak_builds.back();
+               }
                        
-                       if (pbr && pbr->frame + pbr->cnt == oldlen) {
-                               
-                               /* the last PBR extended to the start of the current write,
-                                  so just extend it again.
-                               */
-
-                               pbr->cnt += cnt;
-                       } else {
-                               pending_peak_builds.push_back (new PeakBuildRecord (oldlen, cnt));
-                       }
+               if (pbr && pbr->frame + pbr->cnt == oldlen) {
                        
-                       _peaks_built = false;
+                       /* the last PBR extended to the start of the current write,
+                          so just extend it again.
+                       */
+                       pbr->cnt += cnt;
+               } else {
+                       pending_peak_builds.push_back (new PeakBuildRecord (oldlen, cnt));
+               }
+
+               _peaks_built = false;
        }
        
        
        if (_build_peakfiles) {
-               queue_for_peaks (shared_from_this ());
+               queue_for_peaks (shared_from_this (), false);
        }
 
        _write_data_count = cnt;
@@ -540,7 +539,7 @@ SndFileSource::destructive_write_unlocked (Sample* data, nframes_t cnt)
        }
 
        if (_build_peakfiles) {
-               queue_for_peaks (shared_from_this ());
+               queue_for_peaks (shared_from_this (), true);
        }
        
        return cnt;