View Issue Details

IDProjectCategoryView StatusLast Update
0004207ardourbugspublic2020-04-15 02:02
Reporterx42 Assigned Tox42  
PrioritynormalSeverityminorReproducibilityN/A
Status closedResolutionfixed 
Target Version3.X 
Summary0004207: midi "patch changes" are inaccesible
DescriptionMidi Patch Changes (program/bank)

 - can not be removed by pressing "del" - but shift+right-click works.

 - don't show up in the MIDI-list-editor.

 - may graphically overlap with a track below (switch track-height to "small").

 - they disappear completely (rather than being grayed-out) when the channel is hidden using the midi-channel-dial below the track-name)

 - Right-Clicking on them shows an empty context-menu.

 - The MIDI-channel of the patch-change-event can not be set when creating a new event when using the main-menu, or midi-track-context-menu. But when editing the event (ctrl+right-click) a channel-entry appears.

 - the midi-channel-chooser (pressing 'c') does not work on them (they can't be selected as midi-notes can)


All in all: these are really minor issues, but the overall user-experience is somewhat annoying and unintuitive.
TagsNo tags attached.

Activities

x42

2011-07-19 22:00

administrator   ~0011169

Oh and I forgot to mention: the numbering is always 1 off.

Bank "0" is displayed as "1" in ardour. same for Program.
instead of program range 0-127; ardour has 1-128

x42

2011-07-28 08:58

administrator   ~0011229

attached patch
 - introduces an option to choose if bank/program display starts at zero or one
 - adds channel number to verbose-cursor info
 - solves the graphical overlap on small track heights by displaying bank and program on the same line.

2011-07-28 08:58

 

a3_r9939_midi_patch_display.diff (4,835 bytes)   
diff --git a/gtk2_ardour/midi_region_view.cc b/gtk2_ardour/midi_region_view.cc
index 2d7cccb..fde3ed4 100644
--- a/gtk2_ardour/midi_region_view.cc
+++ b/gtk2_ardour/midi_region_view.cc
@@ -80,6 +80,8 @@ using namespace Editing;
 using namespace ArdourCanvas;
 using Gtkmm2ext::Keyboard;
 
+#define MIDI_BP_ZERO ((Config->get_first_midi_bank_is_zero())?0:1)
+
 MidiRegionView::MidiRegionView (ArdourCanvas::Group *parent, RouteTimeAxisView &tv,
                                 boost::shared_ptr<MidiRegion> r, double spu, Gdk::Color const & basic_color)
 	: RegionView (parent, tv, r, spu, basic_color)
@@ -112,6 +114,7 @@ MidiRegionView::MidiRegionView (ArdourCanvas::Group *parent, RouteTimeAxisView &
 	_note_group->raise_to_top();
 	PublicEditor::DropDownKeys.connect (sigc::mem_fun (*this, &MidiRegionView::drop_down_keys));
 
+	Config->ParameterChanged.connect (*this, invalidator (*this), ui_bind (&MidiRegionView::parameter_changed, this, _1), gui_context());
 	connect_to_diskstream ();
 }
 
@@ -148,6 +151,16 @@ MidiRegionView::MidiRegionView (ArdourCanvas::Group *parent, RouteTimeAxisView &
 	connect_to_diskstream ();
 }
 
+void
+MidiRegionView::parameter_changed (std::string const & p)
+{
+	if (p == "diplay-first-midi-bank-as-zero") {
+		if (_enable_display) {
+			redisplay_model();
+		}
+	}
+}
+
 MidiRegionView::MidiRegionView (const MidiRegionView& other)
 	: sigc::trackable(other)
 	, RegionView (other)
@@ -269,6 +282,7 @@ MidiRegionView::init (Gdk::Color const & basic_color, bool wfd)
 	                                       ui_bind(&MidiRegionView::snap_changed, this),
 	                                       gui_context());
 
+	Config->ParameterChanged.connect (*this, invalidator (*this), ui_bind (&MidiRegionView::parameter_changed, this, _1), gui_context());
 	connect_to_diskstream ();
 }
 
@@ -1158,6 +1172,7 @@ MidiRegionView::display_patch_changes ()
 		if (chn_mask & (1<<i)) {
 			display_patch_changes_on_channel (i);
 		}
+		/* TODO gray-out patch instad of not displaying it */
 	}
 }
 
@@ -1180,8 +1195,8 @@ MidiRegionView::display_patch_changes_on_channel (uint8_t channel)
 			add_canvas_patch_change (*i, patch->name());
 		} else {
 			char buf[16];
-			/* program and bank numbers are zero-based: convert to one-based */
-			snprintf (buf, 16, "%d\n%d", (*i)->program() + 1, (*i)->bank() + 1);
+			/* program and bank numbers are zero-based: convert to one-based: MIDI_BP_ZERO*/
+			snprintf (buf, 16, "%d %d", (*i)->program() + MIDI_BP_ZERO , (*i)->bank() + MIDI_BP_ZERO);
 			add_canvas_patch_change (*i, buf);
 		}
 	}
@@ -2956,7 +2971,7 @@ MidiRegionView::patch_entered (ArdourCanvas::CanvasPatchChange* ev)
 {
 	ostringstream s;
 	/* XXX should get patch name if we can */
-	s << _("Bank:") << (ev->patch()->bank() + 1) << '\n' << _("Program:") << ((int) ev->patch()->program() + 1);
+	s << _("Bank:") << (ev->patch()->bank() + MIDI_BP_ZERO) << '\n' << _("Program:") << ((int) ev->patch()->program()) + MIDI_BP_ZERO << '\n' << _("Channel:") << ((int) ev->patch()->channel() + 1);
 	show_verbose_cursor (s.str(), 10, 20);
 }
 
diff --git a/gtk2_ardour/midi_region_view.h b/gtk2_ardour/midi_region_view.h
index 418cf21..7bdf18f 100644
--- a/gtk2_ardour/midi_region_view.h
+++ b/gtk2_ardour/midi_region_view.h
@@ -299,6 +299,8 @@ protected:
 
 	void reset_width_dependent_items (double pixel_width);
 
+  void parameter_changed (std::string const & p);
+
 private:
 
 	friend class EditNoteDialog;
diff --git a/gtk2_ardour/rc_option_editor.cc b/gtk2_ardour/rc_option_editor.cc
index 72ef75f..09c3c2e 100644
--- a/gtk2_ardour/rc_option_editor.cc
+++ b/gtk2_ardour/rc_option_editor.cc
@@ -1419,6 +1419,14 @@ RCOptionEditor::RCOptionEditor ()
 		     -1, 65536, 1, 10
 		     ));
 
+	add_option (_("MIDI control"),
+		    new BoolOption (
+			    "diplay-first-midi-bank-as-zero",
+			    _("Display first MIDI Bank/Program as 0"),
+			    sigc::mem_fun (*_rc_config, &RCConfiguration::get_first_midi_bank_is_zero),
+			    sigc::mem_fun (*_rc_config, &RCConfiguration::set_first_midi_bank_is_zero)
+			    ));
+
 	/* CONTROL SURFACES */
 
 	add_option (_("Control surfaces"), new ControlSurfacesOptions (*this));
diff --git a/libs/ardour/ardour/rc_configuration_vars.h b/libs/ardour/ardour/rc_configuration_vars.h
index 6fb6759..5693bac 100644
--- a/libs/ardour/ardour/rc_configuration_vars.h
+++ b/libs/ardour/ardour/rc_configuration_vars.h
@@ -44,6 +44,7 @@ CONFIG_VARIABLE (int32_t, mmc_receive_device_id, "mmc-receive-device-id", 0x7f)
 CONFIG_VARIABLE (int32_t, mmc_send_device_id, "mmc-send-device-id", 0)
 CONFIG_VARIABLE (int32_t, initial_program_change, "initial-program-change", -1)
 CONFIG_VARIABLE (int, mtc_qf_speed_tolerance, "mtc-qf-speed-tolerance", 5)
+CONFIG_VARIABLE (bool, first_midi_bank_is_zero, "diplay-first-midi-bank-as-zero", false)
 
 /* control surfaces */
 

x42

2011-07-28 09:06

administrator   ~0011230

Note: the config option is in Edit->Preferences->Midi Control

Semantically it may better fit into "Editor" or "Misc" - But "MIDI [Control]" is the place that catches the eye when one would be looking for that option..

x42

2011-07-28 11:59

administrator   ~0011231

Dave requested to include octal numbering for old synths: e.g.
http://www.pallium.com/bryan/dw8000.php#TapeDumps

Paul's gut feeling suggests to eventually use MIDNAM for this.

more info on ardour-dev -- July 28 2011
"Re: [Ardour-Dev] Fwd: [Ardour-Cvs] r9939 - ardour2/branches/3.0/gtk2_ardour"

cth103

2011-08-03 11:15

administrator   ~0011242

Patch applied to SVN 9948. Thanks!

cth103

2012-06-08 12:18

administrator   ~0013415

Patch changes are greyed out rather than disappeared in SVN 12600.

cth103

2012-06-08 12:28

administrator   ~0013416

I can't reproduce the graphical overlap any more; the empty context menu on right-click is because there is no MIDNAM file selected for the track, and now seems to be fixed; creating a new patch change now has a channel option.

The remanining problems are due to the fact that the patch changes can't be selected like notes, which needs deeper fixes which are unlikely to happen before 3.0, so I'll bump this to 3.X to fix:

selection of patch changes like notes
addition of patch changes to the MIDI list editor (perhaps optionally)

paul

2012-06-11 12:38

administrator   ~0013466

more work is ongoing on this stuff right now, to integrate MIDNAM files with plugin instrument program names (which are, frankly, going to be more common than external h/w).

paul

2012-06-13 11:14

administrator   ~0013492

current 3.0 SVN should be massively improved in all areas relating to program changes.

after some research, it turns out that the idea of mapping plugin presets to program changes is actually very, very hard, and often impossible. there are many plugins that do not respond to MIDI program changes, or do not respond by changing presets; there is confusion over the concepts of "preset" vs. "program" (or "patch"). i've done a bit to provide some support for a token mapping. at present, all plugin types are hard coded as supporting General MIDI, which will change very soon. i have no idea how any code could ever determine that a plugin instrument supports GM, or even whether sending it a PC message changes its current program/state.

still to do: Provide a General MIDI midnam file.

paul

2012-06-16 03:11

administrator   ~0013551

there is now a general MIDI file (device name "Generic", mode = "General MIDI")

i consider this bug fixed, but would appreciate feedback.

paul

2012-06-16 12:31

administrator   ~0013552

actually, now that i read through again, i see that many of these issues are still issues ...

paul

2012-06-16 12:35

administrator   ~0013553

ok, so sub-issue by sub-issue:

 - can not be removed by pressing "del" - but shift+right-click works.

OPEN

 - don't show up in the MIDI-list-editor.

OPEN

 - may graphically overlap with a track below (switch track-height to "small").

FIXED

 - they disappear completely (rather than being grayed-out) when the channel is hidden using the midi-channel-dial below the track-name)

FIXED

 - Right-Clicking on them shows an empty context-menu.

FIXED

 - The MIDI-channel of the patch-change-event can not be set when creating a new event when using the main-menu, or midi-track-context-menu. But when editing the event (ctrl+right-click) a channel-entry appears.

"FIXED" ... just as when adding notes, the first active channel for the track will be used.

 - the midi-channel-chooser (pressing 'c') does not work on them (they can't be selected as midi-notes can)

There are no operations that can be used on notes and patch changes at the same time. The MIDI "selection" consists only of notes. This is a post-3.0 issue.

x42

2012-06-18 17:09

administrator   ~0013568

Wooha. Lots of activity here. Well done.

The first issue (removed by pressing "del") is now invalid, as you said: they can't be selected in the first place.

Having them show up in the MIDI-list editor is still a wishlist item.

Related to that: right-click->[RegionName]->Properties currently does nothing and right-click->[RegionName]->MIDI->List Editor does nothing either! No windows pop up. (svn rev. 12739)

Other things:
  * Quantize does not have any effect the CCs (maybe that's intentional).
  * the "display first midi program as 0" option affects only the mouse-over-info-text and not the value written inside the blue-box on the timeline (which always starts a zero).

paul

2012-06-19 01:26

administrator   ~0013585

the delete key now works, while the mouse is inside a patch change.

appearing in the MIDI list editor is a post-3.0 item.

quantizing non-note data is an interesting issue. will have to investigate the feasibility and semantics for 3.0 or beyond.

x42

2012-06-19 01:35

administrator   ~0013586

re quantization: CC's probably don't need to be quantized, but the order must be preserved.

During some tests here, notes that were originally after a program-change were moved to before the that program-change...

BenLoftis

2015-01-29 02:28

developer   ~0016299

"Delete" key inside a patch change does not work for me.

Patch changes need to be selectable ( and get a red border like other selected things ), so they can be deleted using existing conventions.

More importantly: they need to ignore mouse scrolling unless they are selected (just like midi notes do). Because otherwise you can be scrolling in the canvas, and the scrolling stops if you scroll into a patch change (and you start changing the patch!)

BenLoftis

2015-01-29 02:31

developer   ~0016300

When starting playback, the most-recent patch-change should be applied. I realize this might be a contentious issue but without this feature, starting playback at an arbitrary position means that you often don't hear the correct sound. You have to search for the previous patch change and "roll through" it, then return to where you were working.

This is probably a pain since patch changes are embedded in regions.

paul

2015-01-29 02:57

administrator   ~0016301

las: Ben_at_home: re: your last comment in patch changes ...
[9:55pm] las: Ben_at_home: not just contentious, but part of a much larger and complex task, and should not be done in isolation
[9:55pm] las: Ben_at_home: "correct MIDI state" at a given point in time is far more than the current patch change
[9:55pm] las: Ben_at_home: and really requires a complete pseudo-replay of all MIDI data since session start/zero

drobilla

2015-03-14 21:35

developer   ~0016439

There is another request for the same feature for CCs. In these two cases, I don't think it actually needs a replay: we can quickly look up the relevant PC (the previous one), or current CC value (as with any other automation line). Within regions/sources I think that's pretty easy.

NRPN and such makes it harder, but since we don't support that properly anyway, well...

Considering multiple overlapping regions, though, seems a lot harder. We'd need (?) to inject current state when transitioning to a new region, even when not changing transport position/speed, to get this effect consistently.

Also, different programs in transparent overlapping MIDI regions seems like a fundamentally nonsensical situation.

x42

2020-04-15 02:02

administrator   ~0021352

9 years later: The only remaining issue in Arodur-6.0-pre1 is that patch-changes
"can not be removed by pressing "del" - but shift+right-click works."

Since it's been like this for 9 years, and no other user complained. It must be fine :)

Issue History

Date Modified Username Field Change
2011-07-19 21:57 x42 New Issue
2011-07-19 22:00 x42 Note Added: 0011169
2011-07-19 22:56 cth103 cost => 0.00
2011-07-19 22:56 cth103 Target Version => 3.0-beta1
2011-07-28 08:58 x42 Note Added: 0011229
2011-07-28 08:58 x42 File Added: a3_r9939_midi_patch_display.diff
2011-07-28 09:06 x42 Note Added: 0011230
2011-07-28 11:59 x42 Note Added: 0011231
2011-08-03 11:15 cth103 Note Added: 0011242
2011-11-15 15:15 cth103 Target Version 3.0-beta1 => 3.0
2012-06-08 12:18 cth103 Note Added: 0013415
2012-06-08 12:28 cth103 Note Added: 0013416
2012-06-08 12:28 cth103 Target Version 3.0 => 3.X
2012-06-11 12:38 paul Note Added: 0013466
2012-06-13 11:14 paul Note Added: 0013492
2012-06-13 11:14 paul Status new => feedback
2012-06-13 15:35 cth103 Status feedback => acknowledged
2012-06-16 03:11 paul Note Added: 0013551
2012-06-16 12:31 paul Note Added: 0013552
2012-06-16 12:35 paul Note Added: 0013553
2012-06-18 17:09 x42 Note Added: 0013568
2012-06-19 01:26 paul Note Added: 0013585
2012-06-19 01:35 x42 Note Added: 0013586
2015-01-29 02:28 BenLoftis Note Added: 0016299
2015-01-29 02:31 BenLoftis Note Added: 0016300
2015-01-29 02:57 paul Note Added: 0016301
2015-03-14 21:35 drobilla Note Added: 0016439
2020-04-15 02:02 x42 Assigned To => x42
2020-04-15 02:02 x42 Status acknowledged => closed
2020-04-15 02:02 x42 Resolution open => fixed
2020-04-15 02:02 x42 Note Added: 0021352