View Issue Details

IDProjectCategoryView StatusLast Update
0004657ardourbugspublic2015-09-18 15:19
Reportercolinf Assigned Tocth103  
PrioritynormalSeveritytrivialReproducibilityalways
Status closedResolutionfixed 
Target Version3.0 
Summary0004657: 'Dis:' time remaining indicator shows '00h:00m:00s' on samba share mounted via gvfs
DescriptionI suppose however ardour determines the free space doesn't work for some samba or gvfs-related reason, and I suppose there's nothing to be done about that, but it'd maybe be nicer to show 'unknown' or something similar for the time remaining in that case.
TagsNo tags attached.

Activities

colinf

2012-01-26 13:49

updater   ~0012674

Although the call to statfs() in Session::refresh_disk_space () appears to succeed for smb shares mounted by gvfs, the value of statfsbuf.f_bavail it returns is 0.

I suppose this could be considered a bug in samba or gvfs rather than ardour, but according to the man page for statfs(), "Fields that are undefined for a particular file system are set to 0", so maybe ardour should treat 0 as 'unknown' rather than 'no space' (at least if the file system is not read-only).

2012-06-08 15:56

 

4657.patch (2,290 bytes)   
diff --git a/gtk2_ardour/ardour_ui.cc b/gtk2_ardour/ardour_ui.cc
index c210926..0650985 100644
--- a/gtk2_ardour/ardour_ui.cc
+++ b/gtk2_ardour/ardour_ui.cc
@@ -1054,7 +1054,10 @@ ARDOUR_UI::update_disk_space()
 	char buf[64];
 	framecnt_t fr = _session->frame_rate();
 
-	if (frames == max_framecnt) {
+	if (frames == 0) {
+		/* Available space is unknown */
+		snprintf (buf, sizeof (buf), "%s", _("Disk: <span foreground=\"green\">?</span>"));
+	} else if (frames == max_framecnt) {
 		snprintf (buf, sizeof (buf), "%s", _("Disk: <span foreground=\"green\">24hrs+</span>"));
 	} else {
 		rec_enabled_streams = 0;
diff --git a/libs/ardour/ardour/session.h b/libs/ardour/ardour/session.h
index d31d1f9..3362d93 100644
--- a/libs/ardour/ardour/session.h
+++ b/libs/ardour/ardour/session.h
@@ -1328,7 +1328,7 @@ class Session : public PBD::StatefulDestructible, public PBD::ScopedConnectionLi
 	/* S/W RAID */
 
 	struct space_and_path {
-		uint32_t blocks; /* 4kB blocks */
+		uint32_t blocks; /* 4kB blocks, or 0 if unknown */
 		std::string path;
 
 		space_and_path() {
@@ -1346,6 +1346,7 @@ class Session : public PBD::StatefulDestructible, public PBD::ScopedConnectionLi
 
 	std::vector<space_and_path> session_dirs;
 	std::vector<space_and_path>::iterator last_rr_session_dir;
+	/** If this is 0, we treat it as unknown */
 	uint32_t _total_free_4k_blocks;
 	Glib::Mutex space_lock;
 
diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc
index 559ca64..00a4d68 100644
--- a/libs/ardour/session.cc
+++ b/libs/ardour/session.cc
@@ -3501,6 +3501,9 @@ Session::graph_reordered ()
 	}
 }
 
+/** @return Number of frames that there is disk space for, or 0
+ *  if the count is unknown.
+ */
 framecnt_t
 Session::available_capture_duration ()
 {
diff --git a/libs/ardour/session_state.cc b/libs/ardour/session_state.cc
index e729a7b..3b1e80f 100644
--- a/libs/ardour/session_state.cc
+++ b/libs/ardour/session_state.cc
@@ -2094,6 +2094,10 @@ Session::refresh_disk_space ()
 
 		scale = statfsbuf.f_bsize/4096.0;
 
+		/* f_bavail can be 0 if it is undefined for whatever
+		   filesystem we are looking at; Samba shares mounted
+		   via GVFS are an example of this.
+		*/
 		(*i).blocks = (uint32_t) floor (statfsbuf.f_bavail * scale);
 		_total_free_4k_blocks += (*i).blocks;
 	}
4657.patch (2,290 bytes)   

cth103

2012-06-08 15:56

administrator   ~0013423

Does the attached work ok for you?

colinf

2012-06-08 16:07

updater   ~0013425

Last edited: 2012-06-08 16:07

I haven't tried it yet, but it looks very plausibly like it'll do the right thing for sessions on my NAS.

What happens, though, if statfsbuf.f_bavail == 0 because the file system is read-only (or full), rather than being undefined? I guess we would still want to show "00h:00m:00s" in that case.

2012-06-08 18:05

 

4657.2.patch (6,090 bytes)   
diff --git a/gtk2_ardour/ardour_ui.cc b/gtk2_ardour/ardour_ui.cc
index c210926..ff5659e 100644
--- a/gtk2_ardour/ardour_ui.cc
+++ b/gtk2_ardour/ardour_ui.cc
@@ -1050,16 +1050,21 @@ ARDOUR_UI::update_disk_space()
 		return;
 	}
 
-	framecnt_t frames = _session->available_capture_duration();
+	boost::optional<framecnt_t> opt_frames = _session->available_capture_duration();
 	char buf[64];
 	framecnt_t fr = _session->frame_rate();
 
-	if (frames == max_framecnt) {
+	if (!opt_frames) {
+		/* Available space is unknown */
+		snprintf (buf, sizeof (buf), "%s", _("Disk: <span foreground=\"green\">?</span>"));
+	} else if (opt_frames.get_value_or (0) == max_framecnt) {
 		snprintf (buf, sizeof (buf), "%s", _("Disk: <span foreground=\"green\">24hrs+</span>"));
 	} else {
 		rec_enabled_streams = 0;
 		_session->foreach_route (this, &ARDOUR_UI::count_recenabled_streams);
 
+		framecnt_t frames = opt_frames.get_value_or (0);
+
 		if (rec_enabled_streams) {
 			frames /= rec_enabled_streams;
 		}
diff --git a/libs/ardour/ardour/session.h b/libs/ardour/ardour/session.h
index d31d1f9..55a8ecb 100644
--- a/libs/ardour/ardour/session.h
+++ b/libs/ardour/ardour/session.h
@@ -669,7 +669,7 @@ class Session : public PBD::StatefulDestructible, public PBD::ScopedConnectionLi
 
 	/* s/w "RAID" management */
 
-	framecnt_t available_capture_duration();
+	boost::optional<framecnt_t> available_capture_duration();
 
 	/* I/O bundles */
 
@@ -1328,16 +1328,21 @@ class Session : public PBD::StatefulDestructible, public PBD::ScopedConnectionLi
 	/* S/W RAID */
 
 	struct space_and_path {
-		uint32_t blocks; /* 4kB blocks */
+		uint32_t blocks;     ///< 4kB blocks
+		bool blocks_unknown; ///< true if blocks is unknown
 		std::string path;
 
-		space_and_path() {
-			blocks = 0;
-		}
+		space_and_path ()
+			: blocks (0)
+			, blocks_unknown (true)
+		{}
 	};
 
 	struct space_and_path_ascending_cmp {
 		bool operator() (space_and_path a, space_and_path b) {
+			if (a.blocks_unknown != b.blocks_unknown) {
+				return !a.blocks_unknown;
+			}
 			return a.blocks > b.blocks;
 		}
 	};
@@ -1347,6 +1352,11 @@ class Session : public PBD::StatefulDestructible, public PBD::ScopedConnectionLi
 	std::vector<space_and_path> session_dirs;
 	std::vector<space_and_path>::iterator last_rr_session_dir;
 	uint32_t _total_free_4k_blocks;
+	/** If this is true, _total_free_4k_blocks is not definite,
+	    as one or more of the session directories' filesystems
+	    could not report free space.
+	*/
+	bool _total_free_4k_blocks_uncertain;
 	Glib::Mutex space_lock;
 
 	bool no_questions_about_missing_files;
diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc
index 559ca64..529813f 100644
--- a/libs/ardour/session.cc
+++ b/libs/ardour/session.cc
@@ -143,6 +143,7 @@ Session::Session (AudioEngine &eng,
 	, _all_route_group (new RouteGroup (*this, "all"))
 	, routes (new RouteList)
 	, _total_free_4k_blocks (0)
+	, _total_free_4k_blocks_uncertain (false)
 	, _bundles (new BundleList)
 	, _bundle_xml_node (0)
 	, _current_trans (0)
@@ -3501,9 +3502,16 @@ Session::graph_reordered ()
 	}
 }
 
-framecnt_t
+/** @return Number of frames that there is disk space available to write,
+ *  if known.
+ */
+boost::optional<framecnt_t>
 Session::available_capture_duration ()
 {
+	if (_total_free_4k_blocks_uncertain) {
+		return boost::optional<framecnt_t> ();
+	}
+	
 	float sample_bytes_on_disk = 4.0; // keep gcc happy
 
 	switch (config.get_native_file_data_format()) {
diff --git a/libs/ardour/session_state.cc b/libs/ardour/session_state.cc
index e729a7b..30f8571 100644
--- a/libs/ardour/session_state.cc
+++ b/libs/ardour/session_state.cc
@@ -46,6 +46,10 @@
 #include <sys/mount.h>
 #endif
 
+#ifdef HAVE_SYS_STATVFS_H
+#include <sys/statvfs.h>
+#endif
+
 #include <glib.h>
 
 #include <glibmm.h>
@@ -2079,23 +2083,48 @@ Session::save_template (string template_name)
 void
 Session::refresh_disk_space ()
 {
-#if HAVE_SYS_VFS_H
-	struct statfs statfsbuf;
-	vector<space_and_path>::iterator i;
+#if HAVE_SYS_VFS_H && HAVE_SYS_STATVFS_H
+	
 	Glib::Mutex::Lock lm (space_lock);
-	double scale;
 
 	/* get freespace on every FS that is part of the session path */
 
 	_total_free_4k_blocks = 0;
+	_total_free_4k_blocks_uncertain = false;
 
-	for (i = session_dirs.begin(); i != session_dirs.end(); ++i) {
-		statfs ((*i).path.c_str(), &statfsbuf);
+	for (vector<space_and_path>::iterator i = session_dirs.begin(); i != session_dirs.end(); ++i) {
+
+		struct statfs statfsbuf;
+		statfs (i->path.c_str(), &statfsbuf);
 
-		scale = statfsbuf.f_bsize/4096.0;
+		double const scale = statfsbuf.f_bsize / 4096.0;
 
-		(*i).blocks = (uint32_t) floor (statfsbuf.f_bavail * scale);
-		_total_free_4k_blocks += (*i).blocks;
+		/* See if this filesystem is read-only */
+		struct statvfs statvfsbuf;
+		statvfs (i->path.c_str(), &statvfsbuf);
+
+		/* f_bavail can be 0 if it is undefined for whatever
+		   filesystem we are looking at; Samba shares mounted
+		   via GVFS are an example of this.
+		*/
+		if (statfsbuf.f_bavail == 0) {
+			/* block count unknown */
+			i->blocks = 0;
+			i->blocks_unknown = true;
+		} else if (statvfsbuf.f_flag & ST_RDONLY) {
+			/* read-only filesystem */
+			i->blocks = 0;
+			i->blocks_unknown = false;
+		} else {
+			/* read/write filesystem with known space */
+			i->blocks = (uint32_t) floor (statfsbuf.f_bavail * scale);
+			i->blocks_unknown = false;
+		}
+
+		_total_free_4k_blocks += i->blocks;
+		if (i->blocks_unknown) {
+			_total_free_4k_blocks_uncertain = true;
+		}
 	}
 #endif
 }
diff --git a/libs/ardour/wscript b/libs/ardour/wscript
index ee86a20..2e465e9 100644
--- a/libs/ardour/wscript
+++ b/libs/ardour/wscript
@@ -288,6 +288,7 @@ def configure(conf):
     conf.define('CURRENT_SESSION_FILE_VERSION', CURRENT_SESSION_FILE_VERSION)
 
     conf.check(header_name='sys/vfs.h', define_name='HAVE_SYS_VFS_H',mandatory=False)
+    conf.check(header_name='sys/statvfs.h', define_name='HAVE_SYS_STATVFS_H',mandatory=False)
 
     conf.check(header_name='jack/session.h', uselib = [ 'JACK' ],
                define_name='HAVE_JACK_SESSION')
4657.2.patch (6,090 bytes)   

cth103

2012-06-08 18:05

administrator   ~0013428

True, maybe you could give the .2 version a try. You will need to re-run ./waf configure after patching.

colinf

2012-06-12 15:52

updater   ~0013479

I haven't had a chance to try this on the NAS I first spotted the problem on yet, but it seems to do the right thing on an smb share here at work (shows "?"), and on a read-only SD card (shows "00h:00m:00s").

Maybe "Unknown time" would be better than "?", since there's otherwise nothing to indicate what "Disc:" actually means, but nit-picking aside, the logic seems good.

Also, A3 seems to have a few other minor issues opening sessions on read-only file systems; I suppose I should file a separate bug report for those.

cth103

2012-06-12 16:42

administrator   ~0013482

OK, I've changed the ? to Unknown and applied it. New bug reports would be good for read-only problems. Thanks.

colinf

2015-09-18 15:19

updater   ~0017286

Closing old issues reported by me: these have long since been fixed.

Issue History

Date Modified Username Field Change
2012-01-25 16:20 colinf New Issue
2012-01-26 08:53 cth103 cost => 0.00
2012-01-26 08:53 cth103 Target Version => 3.0
2012-01-26 13:49 colinf Note Added: 0012674
2012-06-08 15:56 cth103 File Added: 4657.patch
2012-06-08 15:56 cth103 Note Added: 0013423
2012-06-08 15:56 cth103 Status new => feedback
2012-06-08 16:07 colinf Note Added: 0013425
2012-06-08 16:07 colinf Note Edited: 0013425
2012-06-08 18:05 cth103 File Added: 4657.2.patch
2012-06-08 18:05 cth103 Note Added: 0013428
2012-06-12 15:52 colinf Note Added: 0013479
2012-06-12 16:42 cth103 Note Added: 0013482
2012-06-12 16:42 cth103 Status feedback => resolved
2012-06-12 16:42 cth103 Resolution open => fixed
2012-06-12 16:42 cth103 Assigned To => cth103
2015-09-18 15:19 colinf Note Added: 0017286
2015-09-18 15:19 colinf Status resolved => closed