View Issue Details

IDCategoryLast Update
0006677bugs2016-02-22 11:44
ReporterEbardieAssigned Totimbyr 
Reproducibilityalways 
Status resolvedResolutionfixed 
PlatformUbuntu VividOSLinuxOS Version3.19.0-32-lowlat
Product Version4.4 
Fixed in Version4.7 
Summary0006677: Post-export script reinterprets timestamp format placeholder giving incorrect filename
DescriptionI'm running a script after exporting a track.

The exported file includes an hours-and-minutes timestamp:

  IDWTLTY_session_2015-11-17_1631.flac


The basename format placeholder (%b) of the output file is passed to my script as:

  IDWTLTY_session_2015-11-17_1633

Note that this is over a minute after the exported filename.

This is a problem when the exported filename is needed in the post-export script.

Does this indicates that the %b is being regenerated after the file has been exported?




  
Steps To ReproduceExport at file that takes longer than a minute to process, with the exported filename set to include the hour-and-minute timestamp, and Export Format Profile's Command to run post-export set to:

  /usr/bin/xterm -hold -e bash -c "echo "%b

Compare the displayed basename with the exported filename.

TagsNo tags attached.

Relationships

related to 0006713 resolvedtimbyr Name of Audio (timestamp) does not match with written Filename in CD-Cue file 

Activities

elgoun

2016-01-13 03:28

reporter   ~0017778

The problem is due to the localtime function and her statically allocated buffer in ExportFilename class.

time_struct variable is set once at construct time, but her value change over time.

Attached a fix that solves the problem.

elgoun

2016-01-13 03:29

reporter  

fix-6677.patch (1,382 bytes)
diff --git a/libs/ardour/ardour/export_filename.h b/libs/ardour/ardour/export_filename.h
index 7eacc11..3761f97 100644
--- a/libs/ardour/ardour/export_filename.h
+++ b/libs/ardour/ardour/export_filename.h
@@ -112,8 +112,7 @@ class LIBARDOUR_API ExportFilename {
 	TimeFormat time_format;
 
 	std::string get_formatted_time (std::string const & format) const;
-	// Due to the static allocation used in strftime(), no destructor or copy-ctor is needed for this
-	struct tm * time_struct;
+	struct tm time_struct;
 
 	ExportTimespanPtr timespan;
 	ExportChannelConfigPtr channel_config;
diff --git a/libs/ardour/export_filename.cc b/libs/ardour/export_filename.cc
index 077106a..f0dda12 100644
--- a/libs/ardour/export_filename.cc
+++ b/libs/ardour/export_filename.cc
@@ -62,7 +62,12 @@ ExportFilename::ExportFilename (Session & session) :
 {
 	time_t rawtime;
 	std::time (&rawtime);
-	time_struct = localtime (&rawtime);
+
+#ifdef PLATFORM_WINDOWS
+	localtime_s (&rawtime, &time_struct);
+#else
+	localtime_r (&rawtime, &time_struct);
+#endif
 
 	folder = session.session_directory().export_path();
 
@@ -319,7 +324,7 @@ string
 ExportFilename::get_formatted_time (string const & format) const
 {
 	char buffer [80];
-	strftime (buffer, 80, format.c_str(), time_struct);
+	strftime (buffer, 80, format.c_str(), &time_struct);
 
 	string return_value (buffer);
 	return return_value;
fix-6677.patch (1,382 bytes)

elgoun

2016-01-13 10:40

reporter   ~0017779

Last edited: 2016-01-13 10:51

View 2 revisions

My first patch is not good.

I saw it after, but there is an implementation of localtime_r in pbd for systems that don't have localtime_r.Futhermore my call to localtime_s (for windows) is bad.

Attached a new patch which is correct (I hope ;-)

It fix 6713 too.

elgoun

2016-01-13 10:40

reporter  

fix-6677-good.patch (1,499 bytes)
diff --git a/libs/ardour/ardour/export_filename.h b/libs/ardour/ardour/export_filename.h
index 7eacc11..3761f97 100644
--- a/libs/ardour/ardour/export_filename.h
+++ b/libs/ardour/ardour/export_filename.h
@@ -112,8 +112,7 @@ class LIBARDOUR_API ExportFilename {
 	TimeFormat time_format;
 
 	std::string get_formatted_time (std::string const & format) const;
-	// Due to the static allocation used in strftime(), no destructor or copy-ctor is needed for this
-	struct tm * time_struct;
+	struct tm time_struct;
 
 	ExportTimespanPtr timespan;
 	ExportChannelConfigPtr channel_config;
diff --git a/libs/ardour/export_filename.cc b/libs/ardour/export_filename.cc
index 077106a..184d8b3 100644
--- a/libs/ardour/export_filename.cc
+++ b/libs/ardour/export_filename.cc
@@ -26,6 +26,7 @@
 #include "pbd/xml++.h"
 #include "pbd/convert.h"
 #include "pbd/enumwriter.h"
+#include "pbd/localtime_r.h"
 
 #include "ardour/libardour_visibility.h"
 #include "ardour/session.h"
@@ -62,7 +63,7 @@ ExportFilename::ExportFilename (Session & session) :
 {
 	time_t rawtime;
 	std::time (&rawtime);
-	time_struct = localtime (&rawtime);
+	localtime_r (&rawtime, &time_struct);
 
 	folder = session.session_directory().export_path();
 
@@ -319,7 +320,7 @@ string
 ExportFilename::get_formatted_time (string const & format) const
 {
 	char buffer [80];
-	strftime (buffer, 80, format.c_str(), time_struct);
+	strftime (buffer, 80, format.c_str(), &time_struct);
 
 	string return_value (buffer);
 	return return_value;
fix-6677-good.patch (1,499 bytes)

timbyr

2016-02-13 03:09

developer   ~0017925

This issue should now be fixed in ardour master as of revision a3dd27c41b or in a nightly build >= 4.6.332

Can you please test and confirm, thanks.

Issue History

Date Modified Username Field Change
2015-11-17 17:15 Ebardie New Issue
2016-01-13 03:28 elgoun Note Added: 0017778
2016-01-13 03:29 elgoun File Added: fix-6677.patch
2016-01-13 10:28 colinf Relationship added related to 0006713
2016-01-13 10:40 elgoun Note Added: 0017779
2016-01-13 10:40 elgoun File Added: fix-6677-good.patch
2016-01-13 10:51 elgoun Note Edited: 0017779 View Revisions
2016-02-13 03:09 timbyr Note Added: 0017925
2016-02-13 03:09 timbyr Assigned To => timbyr
2016-02-13 03:09 timbyr Status new => feedback
2016-02-22 11:44 timbyr Status feedback => resolved
2016-02-22 11:44 timbyr Fixed in Version => 4.7
2016-02-22 11:44 timbyr Resolution open => fixed