View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0006523 | ardour | bugs | public | 2015-08-19 18:08 | 2020-04-19 20:17 |
Reporter | Ejis | Assigned To | x42 | ||
Priority | normal | Severity | major | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Product Version | 5.3 | ||||
Summary | 0006523: (4.2) Serious problems with playlist names | ||||
Description | Hello, First, I had multiple crashes, and when I reopened my session, I found out that regions were erased, and started panicking. Then I looked at the playlists for these regions, and I saw 3 to 10 playlists with the same name. One of them had the regions I was looking for. As I don’t want to mess with the .ardour file too much, I decided, for each track, to give them a unique name to help me recognize them if that happens again. So each track has a main playlist named "lol" (because originality). That’s when it becomes really funny. When I reloaded the session, all the tracks had the same regions! Ardour doesn’t seem to care about IDs, so it thought all the tracks shared the same playlist! So, the problems are: - Ardour can randomly create empty playlists on some tracks, with all the same names, and randomly picks one, so you have to look for the good playlist that actually contains something. - If each track has a playlist with the same name than another track’s playlist, Ardour will think they share the same one, and you’ll get the same regions. Maybe Ardour should prevent the user from giving the same names to different playlists, and, more importantly, looking for IDs rather than names! | ||||
Tags | No tags attached. | ||||
|
fix_6523.patch (4,962 bytes)
diff --git a/libs/ardour/ardour/audio_diskstream.h b/libs/ardour/ardour/audio_diskstream.h index 21a1789..61d0e96 100644 --- a/libs/ardour/ardour/audio_diskstream.h +++ b/libs/ardour/ardour/audio_diskstream.h @@ -227,7 +227,8 @@ class LIBARDOUR_API AudioDiskstream : public Diskstream int use_new_write_source (uint32_t n=0); - int find_and_use_playlist (const std::string &); + int find_and_use_playlist_by_name (const std::string &); + int find_and_use_playlist_by_id (const PBD::ID &); void allocate_temporary_buffers (); diff --git a/libs/ardour/ardour/diskstream.h b/libs/ardour/ardour/diskstream.h index fb9be65..441c312 100644 --- a/libs/ardour/ardour/diskstream.h +++ b/libs/ardour/ardour/diskstream.h @@ -261,7 +261,8 @@ class LIBARDOUR_API Diskstream : public SessionObject, public PublicDiskstream virtual int use_new_write_source (uint32_t n=0) = 0; - virtual int find_and_use_playlist (const std::string&) = 0; + virtual int find_and_use_playlist_by_name (const std::string&) = 0; + virtual int find_and_use_playlist_by_id (const PBD::ID&) = 0; virtual void allocate_temporary_buffers () = 0; diff --git a/libs/ardour/ardour/midi_diskstream.h b/libs/ardour/ardour/midi_diskstream.h index 45f0176..974aab3 100644 --- a/libs/ardour/ardour/midi_diskstream.h +++ b/libs/ardour/ardour/midi_diskstream.h @@ -153,7 +153,8 @@ class LIBARDOUR_API MidiDiskstream : public Diskstream int use_new_write_source (uint32_t n=0); - int find_and_use_playlist (const std::string&); + int find_and_use_playlist_by_name (const std::string&); + int find_and_use_playlist_by_id (const PBD::ID&); void allocate_temporary_buffers (); diff --git a/libs/ardour/audio_diskstream.cc b/libs/ardour/audio_diskstream.cc index 6f7c2d6..173c754 100644 --- a/libs/ardour/audio_diskstream.cc +++ b/libs/ardour/audio_diskstream.cc @@ -245,7 +245,7 @@ AudioDiskstream::get_input_sources () } int -AudioDiskstream::find_and_use_playlist (const string& name) +AudioDiskstream::find_and_use_playlist_by_name (const string& name) { boost::shared_ptr<AudioPlaylist> playlist; @@ -262,6 +262,21 @@ AudioDiskstream::find_and_use_playlist (const string& name) } int +AudioDiskstream::find_and_use_playlist_by_id (const PBD::ID& p_id) +{ + boost::shared_ptr<AudioPlaylist> playlist; + + playlist = boost::dynamic_pointer_cast<AudioPlaylist> (_session.playlists->by_id (p_id)); + + if (!playlist) { + error << string_compose(_("AudioDiskstream: Playlist \"%1\" isn't an audio playlist"), p_id.to_s()) << endmsg; + return -1; + } + + return use_playlist (playlist); +} + +int AudioDiskstream::use_playlist (boost::shared_ptr<Playlist> playlist) { assert(boost::dynamic_pointer_cast<AudioPlaylist>(playlist)); diff --git a/libs/ardour/diskstream.cc b/libs/ardour/diskstream.cc index 7f42140..919dc8f 100644 --- a/libs/ardour/diskstream.cc +++ b/libs/ardour/diskstream.cc @@ -465,6 +465,7 @@ Diskstream::get_state () node->add_property ("flags", enum_2_string (_flags)); node->add_property ("playlist", _playlist->name()); + node->add_property("playlist-id", _playlist->id().to_s()); node->add_property("name", _name); id().print (buf, sizeof (buf)); node->add_property("id", buf); @@ -510,12 +511,23 @@ Diskstream::set_state (const XMLNode& node, int /*version*/) set_align_choice (Automatic, true); } - if ((prop = node.property ("playlist")) == 0) { - return -1; - } + if ((prop = node.property ("playlist-id")) != 0) { + if (find_and_use_playlist_by_id (prop->value())) { + return -1; + } - if (find_and_use_playlist (prop->value())) { - return -1; + } else { + /* + * "Old" Behavior + * Permits "old" sessions to load + */ + if ((prop = node.property ("playlist")) == 0) { + return -1; + } + + if( find_and_use_playlist_by_name (prop->value())) { + return -1; + } } if ((prop = node.property ("speed")) != 0) { diff --git a/libs/ardour/midi_diskstream.cc b/libs/ardour/midi_diskstream.cc index 485967b..207cc07 100644 --- a/libs/ardour/midi_diskstream.cc +++ b/libs/ardour/midi_diskstream.cc @@ -212,7 +212,7 @@ MidiDiskstream::non_realtime_input_change () } int -MidiDiskstream::find_and_use_playlist (const string& name) +MidiDiskstream::find_and_use_playlist_by_name (const string& name) { boost::shared_ptr<MidiPlaylist> playlist; @@ -229,6 +229,21 @@ MidiDiskstream::find_and_use_playlist (const string& name) } int +MidiDiskstream::find_and_use_playlist_by_id (const PBD::ID& p_id) +{ + boost::shared_ptr<MidiPlaylist> playlist; + + playlist = boost::dynamic_pointer_cast<MidiPlaylist> (_session.playlists->by_id (p_id)); + + if (!playlist) { + error << string_compose(_("MidiDiskstream: Playlist \"%1\" isn't a midi playlist"), p_id.to_s()) << endmsg; + return -1; + } + + return use_playlist (playlist); +} + +int MidiDiskstream::use_playlist (boost::shared_ptr<Playlist> playlist) { if (boost::dynamic_pointer_cast<MidiPlaylist>(playlist)) { |
|
I can reproduce this issue. Here is a patch that makes ardour links diskstream to playlist by their ID instead of their name in session file. It is now possible to have playlists with the same name in different tracks. |
|
Great! Thank you! Hope this will end in the official build soon. |
|
The patch looks good and it seems to fix the issue after minimal testing. |
|
Actually I think this patch is more of a stopgap than solution. We have the same issue with all sources e.g embed two external audio-files with the same name from different folders. |
|
This is probably related - recently I did a save as template, and started a new session from template. Tracks in the new session had playlist information from the original session. Not good when you want to record a different song! |
|
x42: Why do you consider the patch a stopgap solution? There may be a similar issue with external audio files but I don't think it is directly related to this issue. |
|
This is still an issue in 5.3. The patch still applies and should fix the issue. Referring to playlists by ID rather than by name still seems like the correct approach to me and not a stopgap. |
|
Indeed the patch looks fine. I got confused by just reading the description. lol. |
|
PS. still: Allowing duplicate names for playlists seems wrong. The name should be unique, and hence lookup by name would work. |
|
I'm not sure I agree that allowing duplicate playlist names is wrong, but if Ardour is going to enforce no duplicate track names then I guess it follows that should also not allow duplicate playlist names. But either way this patch fixes a current issue and I think playlists should be looked up by ID rather than by name as the ID is constant where as the name can change (which might be a source of problems?) |
|
Since the track-name is used as port-name and jack-ports must be unique, there can't be duplicate track-names. Anyway, the issue is more practical: Playlists are displayed in the drop-down menu by name only. It's really confusing if there are 2 or more listed with the same name. |
|
Since Ardour 5.4-470-ge9eea8d it is no longer possible to have two playlists with the same name. |
|
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-08-19 18:08 | Ejis | New Issue | |
2016-01-12 03:56 | elgoun | File Added: fix_6523.patch | |
2016-01-12 04:06 | elgoun | Note Added: 0017774 | |
2016-01-12 04:06 | elgoun | Status | new => confirmed |
2016-01-12 04:44 | elgoun | Note Edited: 0017774 | |
2016-01-12 10:55 | Ejis | Note Added: 0017775 | |
2016-01-15 12:18 | timbyr | Note Added: 0017791 | |
2016-01-15 13:45 | x42 | Note Added: 0017793 | |
2016-01-18 03:44 | mc888 | Note Added: 0017808 | |
2016-02-02 03:39 | timbyr | Note Added: 0017864 | |
2016-09-19 09:59 | timbyr | Note Added: 0018687 | |
2016-09-19 09:59 | timbyr | Product Version | => 5.3 |
2016-10-13 21:27 | x42 | Note Added: 0018811 | |
2016-10-13 21:28 | x42 | Note Added: 0018812 | |
2016-10-14 01:08 | timbyr | Note Added: 0018813 | |
2016-10-14 01:24 | x42 | Note Added: 0018814 | |
2016-11-28 11:17 | x42 | Relationship added | duplicate of 0005681 |
2016-11-28 11:19 | x42 | Note Added: 0019080 | |
2016-11-28 11:19 | x42 | Status | confirmed => resolved |
2016-11-28 11:19 | x42 | Resolution | open => fixed |
2016-11-28 11:19 | x42 | Assigned To | => x42 |
2017-08-02 23:02 | x42 | Relationship added | related to 0007438 |
2020-04-19 20:17 | system | Note Added: 0023508 | |
2020-04-19 20:17 | system | Status | resolved => closed |