Remove some aborts that don't really need to be.

Enforce PatchPrimaryKey sanity at the type level rather than attempting to
check for it everywhere.

Remove dead file.

11 files changed, 75 insertions, 369 deletions

diff --git a/gtk2_ardour/automation_streamview.cc b/gtk2_ardour/automation_streamview.cc
index e504d13931..a336bd6db5 100644
--- a/gtk2_ardour/automation_streamview.cc
+++ b/gtk2_ardour/automation_streamview.cc
@@ -16,7 +16,6 @@
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 */
 
-#include <cassert>
 #include <cmath>
 #include <list>
 #include <utility>
@@ -43,6 +42,8 @@
 #include "rgb_macros.h"
 #include "selection.h"
 
+#include "i18n.h"
+
 using namespace std;
 using namespace ARDOUR;
 using namespace ARDOUR_UI_UTILS;
@@ -71,7 +72,9 @@ AutomationStreamView::~AutomationStreamView ()
 RegionView*
 AutomationStreamView::add_region_view_internal (boost::shared_ptr<Region> region, bool wait_for_data, bool /*recording*/)
 {
-	assert (region);
+	if (!region) {
+		return 0;
+	}
 
 	if (wait_for_data) {
 		boost::shared_ptr<MidiRegion> mr = boost::dynamic_pointer_cast<MidiRegion>(region);
@@ -88,7 +91,10 @@ AutomationStreamView::add_region_view_internal (boost::shared_ptr<Region> region
 	boost::shared_ptr<AutomationList> list;
 	if (control) {
 		list = boost::dynamic_pointer_cast<AutomationList>(control->list());
-		assert(!control->list() || list);
+		if (control->list() && !list) {
+			error << _("unable to display automation region for control without list") << endmsg;
+			return 0;
+		}
 	}
 
 	AutomationRegionView *region_view;
@@ -285,8 +291,9 @@ AutomationStreamView::get_selectables (framepos_t start, framepos_t end, double
 	}
 	for (list<RegionView*>::iterator i = region_views.begin(); i != region_views.end(); ++i) {
 		AutomationRegionView* arv = dynamic_cast<AutomationRegionView*> (*i);
-		assert (arv);
-		arv->line()->get_selectables (start, end, botfrac, topfrac, results);
+		if (arv) {
+			arv->line()->get_selectables (start, end, botfrac, topfrac, results);
+		}
 	}
 }
 
@@ -307,8 +314,9 @@ AutomationStreamView::get_lines () const
 
 	for (list<RegionView*>::const_iterator i = region_views.begin(); i != region_views.end(); ++i) {
 		AutomationRegionView* arv = dynamic_cast<AutomationRegionView*> (*i);
-		assert (arv);
-		lines.push_back (arv->line());
+		if (arv) {
+			lines.push_back (arv->line());
+		}
 	}
 
 	return lines;
diff --git a/gtk2_ardour/canvas_patch_change.cc b/gtk2_ardour/canvas_patch_change.cc
deleted file mode 100644
index 68dbbe3b0d..0000000000
--- a/gtk2_ardour/canvas_patch_change.cc
+++ /dev/null
@@ -1,253 +0,0 @@
-/*
-    Copyright (C) 2000-2010 Paul Davis
-
-    This program is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-
-    This program is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-
-    You should have received a copy of the GNU General Public License
-    along with this program; if not, write to the Free Software
-    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
-
-*/
-
-#include <iostream>
-
-#include <boost/algorithm/string.hpp>
-
-#include "pbd/stacktrace.h"
-
-#include "gtkmm2ext/keyboard.h"
-#include "ardour/instrument_info.h"
-#include "midi++/midnam_patch.h"
-
-#include "ardour_ui.h"
-#include "canvas_patch_change.h"
-#include "editor.h"
-#include "editor_drag.h"
-
-using namespace Gnome::Canvas;
-using namespace MIDI::Name;
-using namespace Gtkmm2ext;
-using namespace std;
-
-/** @param x x position in pixels.
- */
-CanvasPatchChange::CanvasPatchChange(
-	MidiRegionView&                   region,
-	Group&                            parent,
-	const string&                     text,
-	double                            height,
-	double                            x,
-	double                            y,
-	ARDOUR::InstrumentInfo&           info,
-	ARDOUR::MidiModel::PatchChangePtr patch,
-	bool                              active_channel)
-	: CanvasFlag(
-		region,
-		parent,
-		height,
-		(active_channel
-		 ? ARDOUR_UI::config()->get_MidiPatchChangeOutline()
-		 : ARDOUR_UI::config()->get_MidiPatchChangeInactiveChannelOutline()),
-		(active_channel
-		 ? ARDOUR_UI::config()->get_MidiPatchChangeFill()
-		 : ARDOUR_UI::config()->get_MidiPatchChangeInactiveChannelFill()),
-		x,
-		y)
-	, _info (info)
-	, _patch (patch)
-	, _popup_initialized(false)
-{
-	set_text (text);
-}
-
-CanvasPatchChange::~CanvasPatchChange()
-{
-}
-
-void
-CanvasPatchChange::initialize_popup_menus()
-{
-	boost::shared_ptr<ChannelNameSet> channel_name_set = _info.get_patches (_patch->channel());
-
-	if (!channel_name_set) {
-		return;
-	}
-
-	const ChannelNameSet::PatchBanks& patch_banks = channel_name_set->patch_banks();
-
-	if (patch_banks.size() > 1) {
-		// fill popup menu:
-		Gtk::Menu::MenuList& patch_bank_menus = _popup.items();
-		
-		for (ChannelNameSet::PatchBanks::const_iterator bank = patch_banks.begin();
-		     bank != patch_banks.end();
-		     ++bank) {
-			Gtk::Menu& patch_bank_menu = *manage(new Gtk::Menu());
-			
-			const PatchNameList& patches = (*bank)->patch_name_list();
-			Gtk::Menu::MenuList& patch_menus = patch_bank_menu.items();
-			
-			for (PatchNameList::const_iterator patch = patches.begin();
-			     patch != patches.end();
-			     ++patch) {
-				std::string name = (*patch)->name();
-				boost::replace_all (name, "_", " ");
-				
-				patch_menus.push_back(
-					Gtk::Menu_Helpers::MenuElem(
-						name,
-						sigc::bind(sigc::mem_fun(*this, &CanvasPatchChange::on_patch_menu_selected),
-						           (*patch)->patch_primary_key())) );
-			}
-			
-			std::string name = (*bank)->name();
-			boost::replace_all (name, "_", " ");
-			
-			patch_bank_menus.push_back(
-				Gtk::Menu_Helpers::MenuElem(
-					name,
-					patch_bank_menu) );
-		}
-	} else {
-		/* only one patch bank, so make it the initial menu */
-
-		const PatchNameList& patches = patch_banks.front()->patch_name_list();
-		Gtk::Menu::MenuList& patch_menus = _popup.items();
-		
-		for (PatchNameList::const_iterator patch = patches.begin();
-		     patch != patches.end();
-		     ++patch) {
-			std::string name = (*patch)->name();
-			boost::replace_all (name, "_", " ");
-			
-			patch_menus.push_back (
-				Gtk::Menu_Helpers::MenuElem (
-					name,
-					sigc::bind (sigc::mem_fun(*this, &CanvasPatchChange::on_patch_menu_selected),
-					            (*patch)->patch_primary_key())));
-		}
-	}
-}
-	
-void
-CanvasPatchChange::on_patch_menu_selected(const PatchPrimaryKey& key)
-{
-	_region.change_patch_change (*this, key);
-}
-
-static bool
-in_edit_mode(Editor* editor)
-{
-	return (editor->internal_editing() &&
-	        (editor->current_mouse_mode() == Editing::MouseObject ||
-	         editor->current_mouse_mode() == Editing::MouseDraw));
-}
-
-bool
-CanvasPatchChange::on_event (GdkEvent* ev)
-{
-	/* XXX: icky dcast */
-	Editor* e = dynamic_cast<Editor*> (&_region.get_time_axis_view().editor());
-	
-	if (!in_edit_mode(e)) {
-		return false;
-	}
-
-	switch (ev->type) {
-	case GDK_BUTTON_PRESS:
-		if (Gtkmm2ext::Keyboard::is_delete_event (&ev->button)) {
-			
-			_region.delete_patch_change (this);
-			return true;
-			
-		} else if (Gtkmm2ext::Keyboard::is_edit_event (&ev->button)) {
-			
-			_region.edit_patch_change (this);
-			return true;
-			
-		} else if (ev->button.button == 1) {
-			e->drags()->set (new PatchChangeDrag (e, this, &_region), ev);
-			return true;
-		}
-
-		if (ev->button.button == 3) {
-			if (!_popup_initialized) {
-				initialize_popup_menus();
-				_popup_initialized = true;
-			}
-			if (!_popup.items().empty()) {
-				_popup.popup(ev->button.button, ev->button.time);
-			}
-			return true;
-		}
-		break;
-
-	case GDK_KEY_PRESS:
-		switch (ev->key.keyval) {
-		case GDK_Up:
-		case GDK_KP_Up:
-		case GDK_uparrow:
-			if (Keyboard::modifier_state_contains (ev->key.state, Keyboard::PrimaryModifier)) {
-				_region.previous_bank (*this);
-			} else {
-				_region.previous_patch (*this);
-			}
-			break;
-		case GDK_Down:
-		case GDK_KP_Down:
-		case GDK_downarrow:
-			if (Keyboard::modifier_state_contains (ev->key.state, Keyboard::PrimaryModifier)) {
-				_region.next_bank (*this);
-			} else {
-				_region.next_patch (*this);
-			}
-			break;
-		case GDK_Delete:
-		case GDK_BackSpace:
-			_region.delete_patch_change (this);
-			break;
-		default:
-			break;
-		}
-		break;
-
-	case GDK_SCROLL:
-		if (ev->scroll.direction == GDK_SCROLL_UP) {
-			if (Keyboard::modifier_state_contains (ev->scroll.state, Keyboard::PrimaryModifier)) {
-				_region.previous_bank (*this);
-			} else {
-				_region.previous_patch (*this);
-			}
-		} else if (ev->scroll.direction == GDK_SCROLL_DOWN) {
-				if (Keyboard::modifier_state_contains (ev->scroll.state, Keyboard::PrimaryModifier)) {
-					_region.next_bank (*this);
-				} else {
-					_region.next_patch (*this);
-				}
-		}
-		return true;
-
-	case GDK_ENTER_NOTIFY:
-		_region.patch_entered (this);
-		return true;
-		break;
-
-	case GDK_LEAVE_NOTIFY:
-		_region.patch_left (this);
-		return true;
-		break;
-
-	default:
-		break;
-	}
-
-	return false;
-}
diff --git a/gtk2_ardour/midi_region_view.cc b/gtk2_ardour/midi_region_view.cc
index 2879e65af7..11534dd035 100644
--- a/gtk2_ardour/midi_region_view.cc
+++ b/gtk2_ardour/midi_region_view.cc
@@ -1881,15 +1881,11 @@ MidiRegionView::get_patch_key_at (Evoral::MusicalTime time, uint8_t channel, MID
 	}
 
 	if (i != _model->patch_changes().end() && patch_applies(*i, time, channel)) {
-		key.bank_number    = (*i)->bank();
-		key.program_number = (*i)->program ();
+		key.set_bank((*i)->bank());
+		key.set_program((*i)->program ());
 	} else {
-		key.bank_number = key.program_number = 0;
-	}
-
-	if (!key.is_sane()) {
-		error << string_compose(_("insane MIDI patch key %1:%2"),
-		                        key.bank_number, key.program_number) << endmsg;
+		key.set_bank(0);
+		key.set_program(0);
 	}
 }
 
@@ -1898,11 +1894,11 @@ MidiRegionView::change_patch_change (PatchChange& pc, const MIDI::Name::PatchPri
 {
 	MidiModel::PatchChangeDiffCommand* c = _model->new_patch_change_diff_command (_("alter patch change"));
 
-	if (pc.patch()->program() != new_patch.program_number) {
-		c->change_program (pc.patch (), new_patch.program_number);
+	if (pc.patch()->program() != new_patch.program()) {
+		c->change_program (pc.patch (), new_patch.program());
 	}
 
-	int const new_bank = new_patch.bank_number;
+	int const new_bank = new_patch.bank();
 	if (pc.patch()->bank() != new_bank) {
 		c->change_bank (pc.patch (), new_bank);
 	}
@@ -1988,47 +1984,12 @@ MidiRegionView::delete_patch_change (PatchChange* pc)
 }
 
 void
-MidiRegionView::previous_patch (PatchChange& patch)
-{
-	if (patch.patch()->program() < 127) {
-		MIDI::Name::PatchPrimaryKey key = patch_change_to_patch_key (patch.patch());
-		key.program_number++;
-		change_patch_change (patch, key);
-	}
-}
-
-void
-MidiRegionView::next_patch (PatchChange& patch)
-{
-	if (patch.patch()->program() > 0) {
-		MIDI::Name::PatchPrimaryKey key = patch_change_to_patch_key (patch.patch());
-		key.program_number--;
-		change_patch_change (patch, key);
-	}
-}
-
-void
-MidiRegionView::next_bank (PatchChange& patch)
-{
-	if (patch.patch()->program() < 127) {
-		MIDI::Name::PatchPrimaryKey key = patch_change_to_patch_key (patch.patch());
-		if (key.bank_number > 0) {
-			key.bank_number--;
-			change_patch_change (patch, key);
-		}
-	}
-}
-
-void
-MidiRegionView::previous_bank (PatchChange& patch)
+MidiRegionView::step_patch (PatchChange& patch, int dbank, int dprog)
 {
-	if (patch.patch()->program() > 0) {
-		MIDI::Name::PatchPrimaryKey key = patch_change_to_patch_key (patch.patch());
-		if (key.bank_number < 127) {
-			key.bank_number++;
-			change_patch_change (patch, key);
-		}
-	}
+	MIDI::Name::PatchPrimaryKey key = patch_change_to_patch_key(patch.patch());
+	key.set_bank(key.bank() + dbank);
+	key.set_program(key.program() + dprog);
+	change_patch_change(patch, key);
 }
 
 void
@@ -3893,8 +3854,8 @@ MidiRegionView::show_verbose_cursor (boost::shared_ptr<NoteType> n) const
 			get_patch_key_at(n->time(), n->channel(), patch_key);
 			name = device_names->note_name(mtv->gui_property(X_("midnam-custom-device-mode")),
 			                               n->channel(),
-			                               patch_key.bank_number,
-			                               patch_key.program_number,
+			                               patch_key.bank(),
+			                               patch_key.program(),
 			                               n->note());
 		}
 	}
diff --git a/gtk2_ardour/midi_region_view.h b/gtk2_ardour/midi_region_view.h
index a2d364c0d6..a07a46825f 100644
--- a/gtk2_ardour/midi_region_view.h
+++ b/gtk2_ardour/midi_region_view.h
@@ -153,16 +153,8 @@ public:
 
 	void delete_sysex (SysEx*);
 
-	/** Alter a given patch to be its predecessor in the MIDNAM file.
-	 */
-	void previous_patch (PatchChange &);
-
-	/** Alters a given patch to be its successor in the MIDNAM file.
-	 */
-	void next_patch (PatchChange &);
-
-	void previous_bank (PatchChange &);
-	void next_bank (PatchChange &);
+	/** Change a patch to the next or previous bank/program. */
+	void step_patch (PatchChange& patch, int dbank, int dprog);
 
 	/** Displays all patch change events in the region as flags on the canvas.
 	 */
diff --git a/gtk2_ardour/patch_change.cc b/gtk2_ardour/patch_change.cc
index 1701e66b56..844a129b1d 100644
--- a/gtk2_ardour/patch_change.cc
+++ b/gtk2_ardour/patch_change.cc
@@ -199,12 +199,12 @@ PatchChange::event_handler (GdkEvent* ev)
 		case GDK_Up:
 		case GDK_KP_Up:
 		case GDK_uparrow:
-			_region.previous_patch (*this);
+			_region.step_patch (*this, 0, -1);
 			break;
 		case GDK_Down:
 		case GDK_KP_Down:
 		case GDK_downarrow:
-			_region.next_patch (*this);
+			_region.step_patch (*this, 0, 1);
 			break;
 		default:
 			break;
@@ -213,10 +213,10 @@ PatchChange::event_handler (GdkEvent* ev)
 
 	case GDK_SCROLL:
 		if (ev->scroll.direction == GDK_SCROLL_UP) {
-			_region.previous_patch (*this);
+			_region.step_patch (*this, 0, -1);
 			return true;
 		} else if (ev->scroll.direction == GDK_SCROLL_DOWN) {
-			_region.next_patch (*this);
+			_region.step_patch (*this, 0, 1);
 			return true;
 		}
 		break;
diff --git a/gtk2_ardour/patch_change_dialog.cc b/gtk2_ardour/patch_change_dialog.cc
index 23941e1545..9072801c83 100644
--- a/gtk2_ardour/patch_change_dialog.cc
+++ b/gtk2_ardour/patch_change_dialog.cc
@@ -285,7 +285,7 @@ PatchChangeDialog::set_active_patch_combo ()
 		boost::replace_all (n, "_", " ");
 
 		MIDI::Name::PatchPrimaryKey const & key = (*j)->patch_primary_key ();
-		if (key.program_number == _program.get_value() - 1) {
+		if (key.program() == _program.get_value() - 1) {
 			_ignore_signals = true;
 			_patch_combo.set_active_text (n);
 			_ignore_signals = false;
diff --git a/gtk2_ardour/region_view.cc b/gtk2_ardour/region_view.cc
index 678ac4dbaf..bd80e27acb 100644
--- a/gtk2_ardour/region_view.cc
+++ b/gtk2_ardour/region_view.cc
@@ -18,7 +18,6 @@
 */
 
 #include <cmath>
-#include <cassert>
 #include <algorithm>
 
 #include <gtkmm.h>
diff --git a/libs/ardour/ardour/graph.h b/libs/ardour/ardour/graph.h
index b433580a54..dc531b3680 100644
--- a/libs/ardour/ardour/graph.h
+++ b/libs/ardour/ardour/graph.h
@@ -29,7 +29,6 @@
 #include <boost/shared_ptr.hpp>
 
 #include <glib.h>
-#include <cassert>
 
 #include "pbd/semutils.h"
 
diff --git a/libs/evoral/src/ControlList.cpp b/libs/evoral/src/ControlList.cpp
index 902a20980e..8176850428 100644
--- a/libs/evoral/src/ControlList.cpp
+++ b/libs/evoral/src/ControlList.cpp
@@ -331,7 +331,6 @@ ControlList::fast_simple_add (double when, double value)
 	Glib::Threads::Mutex::Lock lm (_lock);
 	/* to be used only for loading pre-sorted data from saved state */
 	_events.insert (_events.end(), new ControlEvent (when, value));
-	assert(_events.back());
 
 	mark_dirty ();
 }
@@ -1017,9 +1016,10 @@ ControlList::truncate_start (double overall_length)
 		double first_legal_value;
 		double first_legal_coordinate;
 
-		assert(!_events.empty());
-
-		if (overall_length == _events.back()->when) {
+		if (_events.empty()) {
+			/* nothing to truncate */
+			return;
+		} else if (overall_length == _events.back()->when) {
 			/* no change in overall length */
 			return;
 		}
diff --git a/libs/midi++2/midi++/midnam_patch.h b/libs/midi++2/midi++/midnam_patch.h
index 4e5bb29f55..5902bd5c05 100644
--- a/libs/midi++2/midi++/midnam_patch.h
+++ b/libs/midi++2/midi++/midnam_patch.h
@@ -21,6 +21,7 @@
 #define MIDNAM_PATCH_H_
 
 #include <algorithm>
+#include <cassert>
 #include <iostream>
 #include <string>
 #include <list>
@@ -43,41 +44,41 @@ namespace Name
 struct LIBMIDIPP_API PatchPrimaryKey
 {
 public:
-	int bank_number;
-	int program_number;
-
-	PatchPrimaryKey (uint8_t a_program_number = 0, uint16_t a_bank_number = 0) {
-		bank_number = std::min (a_bank_number, (uint16_t) 16384);
-		program_number = std::min (a_program_number, (uint8_t) 127);
-	}
-	
-	bool is_sane() const { 	
-		return ((bank_number >= 0) && (bank_number <= 16384) && 
-			(program_number >=0 ) && (program_number <= 127));
-	}
+	PatchPrimaryKey (int program_num = 0, int bank_num = 0)
+		: _bank(std::max(0, std::min(bank_num, 16383)))
+		, _program(std::max(0, std::min(program_num, 127)))
+	{}
 	
 	inline PatchPrimaryKey& operator=(const PatchPrimaryKey& id) {
-		bank_number = id.bank_number;
-		program_number = id.program_number;
+		_bank    = id._bank;
+		_program = id._program;
 		return *this;
 	}
 	
 	inline bool operator==(const PatchPrimaryKey& id) const {
-		return (bank_number == id.bank_number && program_number == id.program_number);
+		return (_bank    == id._bank &&
+		        _program == id._program);
 	}
 	
-	/**
-	 * obey strict weak ordering or crash in STL containers
-	 */
+	/** Strict weak ordering. */
 	inline bool operator<(const PatchPrimaryKey& id) const {
-		if (bank_number < id.bank_number) {
+		if (_bank < id._bank) {
 			return true;
-		} else if (bank_number == id.bank_number && program_number < id.program_number) {
+		} else if (_bank == id._bank && _program < id._program) {
 			return true;
 		}
-		
 		return false;
 	}
+
+	void set_bank(int bank)       { _bank    = std::max(0, std::min(bank, 16383)); }
+	void set_program(int program) { _program = std::max(0, std::min(program, 127)); }
+
+	inline uint16_t bank()    const { return _bank; }
+	inline uint8_t  program() const { return _program; }
+
+private:
+	uint16_t _bank;
+	uint8_t  _program;
 };
 
 class PatchBank;
@@ -94,11 +95,11 @@ public:
 	
 	const std::string& note_list_name() const  { return _note_list_name; }
 
-	uint8_t program_number() const     { return _id.program_number; }
-	void set_program_number(uint8_t n) { _id.program_number = n; }
+	uint8_t program_number() const     { return _id.program(); }
+	void set_program_number(uint8_t n) { _id.set_program(n); }
 
-	uint16_t bank_number() const      { return _id.bank_number; }
-	void set_bank_number (uint16_t n) { _id.bank_number = n; }
+	uint16_t bank_number() const      { return _id.bank(); }
+	void set_bank_number (uint16_t n) { _id.set_bank(n); }
 
 	const PatchPrimaryKey&   patch_primary_key()   const { return _id; }
 
@@ -161,12 +162,10 @@ public:
 	}
 	
 	boost::shared_ptr<Patch> find_patch(const PatchPrimaryKey& key) {
-		assert(key.is_sane());
 		return _patch_map[key];
 	}
 	
 	boost::shared_ptr<Patch> previous_patch(const PatchPrimaryKey& key) {
-		assert(key.is_sane());
 		for (PatchList::const_iterator i = _patch_list.begin();
 			 i != _patch_list.end();
 			 ++i) {
@@ -182,7 +181,6 @@ public:
 	}
 	
 	boost::shared_ptr<Patch> next_patch(const PatchPrimaryKey& key) {
-		assert(key.is_sane());
 		for (PatchList::const_iterator i = _patch_list.begin();
 			 i != _patch_list.end();
 			 ++i) {
diff --git a/libs/midi++2/midnam_patch.cc b/libs/midi++2/midnam_patch.cc
index 77e13f29e1..e69842e6cd 100644
--- a/libs/midi++2/midnam_patch.cc
+++ b/libs/midi++2/midnam_patch.cc
@@ -58,7 +58,8 @@ static int
 initialize_primary_key_from_commands (
 	const XMLTree& tree, PatchPrimaryKey& id, const XMLNode* node)
 {
-	id.bank_number = 0;
+	uint16_t bank    = 0;
+	uint8_t  program = 0;
 
 	const XMLNodeList events = node->children();
 	for (XMLNodeList::const_iterator i = events.begin(); i != events.end(); ++i) {
@@ -69,18 +70,19 @@ initialize_primary_key_from_commands (
 			const string& value   = node->property("Value")->value();
 
 			if (control == "0") {
-				id.bank_number |= string_to_int(tree, value) << 7;
+				bank |= string_to_int(tree, value) << 7;
 			} else if (control == "32") {
-				id.bank_number |= string_to_int(tree, value);
+				bank |= string_to_int(tree, value);
 			}
 
 		} else if (node->name() == "ProgramChange") {
 			const string& number = node->property("Number")->value();
 			assert(number != "");
-			id.program_number = string_to_int(tree, number);
+			program = string_to_int(tree, number);
 		}
 	}
 
+	id = PatchPrimaryKey(program, bank);
 	return 0;
 }
 
@@ -91,7 +93,7 @@ Patch::get_state (void)
 
 	/* XXX this is totally wrong */
 
-	node->add_property("Number", string_compose ("%1", _id.program_number));
+	node->add_property("Number", string_compose ("%1", _id.program()));
 	node->add_property("Name",   _name);
 
 	/*
@@ -120,7 +122,7 @@ Patch::set_state (const XMLTree& tree, const XMLNode& node)
 
 	const XMLProperty* program_change = node.property("ProgramChange");
 	if (program_change) {
-		_id.program_number = string_to_int(tree, program_change->value());
+		_id = PatchPrimaryKey(string_to_int(tree, program_change->value()), _id.bank());
 	}
 
 	const XMLProperty* name = node.property("Name");
@@ -426,7 +428,7 @@ PatchBank::set_state (const XMLTree& tree, const XMLNode& node)
 		if (initialize_primary_key_from_commands (tree, id, commands)) {
 			return -1;
 		}
-		_number = id.bank_number;
+		_number = id.bank();
 	}
 
 	XMLNode* patch_name_list = node.child("PatchNameList");