View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0006677 | ardour | bugs | public | 2015-11-17 17:15 | 2020-04-19 20:17 |
| Reporter | Ebardie | Assigned To | timbyr | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Platform | Ubuntu Vivid | OS | Linux | OS Version | 3.19.0-32-lowlat |
| Product Version | 4.4 | ||||
| Fixed in Version | 4.7 | ||||
| Summary | 0006677: Post-export script reinterprets timestamp format placeholder giving incorrect filename | ||||
| Description | I'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 Reproduce | Export 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. | ||||
| Tags | No tags attached. | ||||
|
|
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. |
|
|
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;
|
|
|
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. |
|
|
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;
|
|
|
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 has been closed automatically, by Trigger Close Plugin. Feel free to re-open with additional information if you think the issue is not resolved. |
| 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 | |
| 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 |
| 2020-04-19 20:17 | system | Note Added: 0023561 | |
| 2020-04-19 20:17 | system | Status | resolved => closed |