[Audacity-devel] Fixing EXPERIMENTAL_MIDI

Discussion:

[Audacity-devel] Fixing EXPERIMENTAL_MIDI_OUT

James Crook

2017-02-11 10:27:43 UTC

Hi Poke,

This is great to hear.

Changes that make EXPERIMENTAL_MIDI_OUT work well, and that don't affect
Audacity when it is not defined are very welcome.

Moving to defining EXPERIMENTAL_MIDI_OUT by default in Audacity is
likely to take a lot more work than that. I'm seeing code for example
that uses mLeftTrack in standard Audacity and mTrack with
EXPERIMENTAL_MIDI_OUT. That suggests to me that there will be issues
with stereo mixing with EXPERIMENTAL_MIDI_OUT....

If EXPERIMENTAL_MIDI_OUT is to become mainstream we need to be confident
in its stability. Probably it won't go live until we have moved a lot
of it into its own files - refactoring it out of TrackPanel, AudioIO and
MixerBoard, so we can see it more independently of the other code.
Exactly how needs discussion, so I am glad you came here and asked
rather than just sending pull requests.

One possibility when EXPERIMENTAL_MIDI_OUT is further along is that we
could distribute experimental builds with it enabled. This would give us
user feedback on stability and issues.

Thanks for getting in contact.

--James.

Hey everyone.
I've recently been working on fixing EXPERIMENTAL_MIDI_OUT. Initially
I just started because I wanted to clean up the UX for toggling midi
channels (which could be done even when the buttons weren't rendered
since EXPERIMENTAL_MIDI_OUT wasn't enabled), but when I found out how
close this feature was to functional, I decided I want to get it fully
up to date.
You can see my first attempt on PR #177
(https://github.com/audacity/audacity/pull/177); I've closed that
though because it's not very good and doesn't cover all cases (plus it
still has a lot of the code hackilly written). I'm currently trying
again with a more general rework (with the same general ideas as that
PR, but done somewhat better).
If there's anything that I should know regarding EXPERIMENTAL_MIDI_OUT
(or comments regarding that PR), I'd very much appreciate hearing
them, thanks!
--Poke
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel

Steve the Fiddle

2017-02-11 10:58:24 UTC

Permalink

Great to see some work on MIDI.

Steve

Post by James Crook
Hi Poke,
This is great to hear.
Changes that make EXPERIMENTAL_MIDI_OUT work well, and that don't affect
Audacity when it is not defined are very welcome.
Moving to defining EXPERIMENTAL_MIDI_OUT by default in Audacity is
likely to take a lot more work than that. I'm seeing code for example
that uses mLeftTrack in standard Audacity and mTrack with
EXPERIMENTAL_MIDI_OUT. That suggests to me that there will be issues
with stereo mixing with EXPERIMENTAL_MIDI_OUT....
If EXPERIMENTAL_MIDI_OUT is to become mainstream we need to be confident
in its stability. Probably it won't go live until we have moved a lot
of it into its own files - refactoring it out of TrackPanel, AudioIO and
MixerBoard, so we can see it more independently of the other code.
Exactly how needs discussion, so I am glad you came here and asked
rather than just sending pull requests.
One possibility when EXPERIMENTAL_MIDI_OUT is further along is that we
could distribute experimental builds with it enabled. This would give us
user feedback on stability and issues.
Thanks for getting in contact.
--James.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel

Roger Dannenberg

2017-02-11 14:52:18 UTC

Permalink

That's great to hear. Only a couple of things come to mind about
EXPERIMENTAL_MIDI_OUT:

* Note Tracks were originally created using the allegro library.
Allegro is both an in-memory representation and an external
text-based file format. Allegro is much more general than MIDI,
supporting "notes" with lists of attribute/value properties, and
"updates" which are time-stamped attribute/value pairs associated
with "notes". Because "values" can be strings, floats, ints, etc.,
values are tagged with type codes and there's a lot of dynamic
allocation. C++ is not ideal for this because in the library there
are lots of cases where an event ("note" or "update") is referenced
from multiple places. If you delete a track and all it's events, you
might create dangling pointers elsewhere, but if you don't delete
the events, you might leak memory. This *could* be solved with smart
pointers, explicit ref. counting, or even garbage collection, but at
least at the time this was all designed, we thought managing "by
hand" would save a lot of memory and not be that difficult -- still,
if you go in with the wrong expectations or simply ignore memory
allocation/deallocation, you won't get very far. On the other hand,
Allegro has some *very* powerful and high-level operations to
support cut, copy, paste, stretch, mapping from time to beats ("time
maps" or "tempo maps"), etc., plus midi file and allegro format
import/export. That was a *lot* of work, much of it pretty tricky
with lots of edge cases.
* There is a lot of code for synchronizing audio to MIDI. PortMidi
includes the ability to provide an external clock for
synchronization. Since audio clocks and system clocks are not
synchronized (your audio sample clock runs off a hardware crystal
oscillator that's separate from whatever the OS uses for time), I
use PortAudio's sample count as a time reference for MIDI. This is
somewhat tricky because audio might not be running, audio starts and
stops, and the OS may not give PortAudio all the info it needs to
compute the current sample position accurately. Nevertheless, I
think the implementation is pretty good. What happens between
Audacity and PortMidi is Audacity generates timestamped MIDI
messages and delivers them to PortMidi more-or-less at the same time
any corresponding audio samples are delivered to PortAudio. PortMidi
then (1) translates the timestamp to system time (which is just
adding an offset), (2) adds a "latency" offset -- where latency is
set to match the audio latency of the system, (3) PortMidi then uses
the MIDI device driver to get the best timing accuracy possible. The
goal is for MIDI messages to be finally delivered just as
corresponding audio samples are hitting the DAC.

Let me know if you have questions - this has been "swapped out" quite
awhile, but it would be cool to get it in production.

-Roger

Pokechu22

2017-02-12 05:02:42 UTC

Permalink

Post by James Crook
Moving to defining EXPERIMENTAL_MIDI_OUT by default in Audacity is

likely to take a lot more work than that. I'm seeing code for example

that uses mLeftTrack in standard Audacity and mTrack with

EXPERIMENTAL_MIDI_OUT. That suggests to me that there will be issues

with stereo mixing with EXPERIMENTAL_MIDI_OUT....

If EXPERIMENTAL_MIDI_OUT is to become mainstream we need to be confident

in its stability. Probably it won't go live until we have moved a lot

of it into its own files - refactoring it out of TrackPanel, AudioIO and

MixerBoard, so we can see it more independently of the other code.

Exactly how needs discussion, so I am glad you came here and asked

rather than just sending pull requests.
// I suggest that when this is no longer experimental, rather than all

Post by James Crook
these #ifdef's,

// this be done by factoring, i.e., add two subclasses to

Post by James Crook
MixerTrackCluster,

// MixerNoteTrackCluster and MixerWaveTrackCluster, such that all the

Post by James Crook
common

// code is in the parent, and these #ifdef's are only around

// MixerNoteTrackCluster rather than sprinkled throughout

Post by James Crook
MixerTrackCluster.

I'm guessing that that would be a reasonable implementation. The mixer
code as-is seems to be fairly broken, such as casting all tracks to
WaveTrack* (and then back to NoteTrack*) which seems to cause things to not
work at all (the cast back seems to not occur). I don't think left/right
is an issue in the current implementation, but it's still ugly and should
be removed if possible.

Also regarding the mixer, what's the point of the MusicalInstrument type
(and related code)? I can't see where that shows up on the actual program,
but it sounds vaguely midi-related (yet, it isn't part of the midi code).

Post by James Crook
That's great to hear. Only a couple of things come to mind about
- Note Tracks were originally created using the allegro library.
Allegro is both an in-memory representation and an external text-based file
format. Allegro is much more general than MIDI, supporting "notes" with
lists of attribute/value properties, and "updates" which are time-stamped
attribute/value pairs associated with "notes". Because "values" can be
strings, floats, ints, etc., values are tagged with type codes and there's
a lot of dynamic allocation. C++ is not ideal for this because in the
library there are lots of cases where an event ("note" or "update") is
referenced from multiple places. If you delete a track and all it's events,
you might create dangling pointers elsewhere, but if you don't delete the
events, you might leak memory. This *could* be solved with smart pointers,
explicit ref. counting, or even garbage collection, but at least at the
time this was all designed, we thought managing "by hand" would save a lot
of memory and not be that difficult -- still, if you go in with the wrong
expectations or simply ignore memory allocation/deallocation, you won't get
very far. On the other hand, Allegro has some *very* powerful and
high-level operations to support cut, copy, paste, stretch, mapping from
time to beats ("time maps" or "tempo maps"), etc., plus midi file and
allegro format import/export. That was a *lot* of work, much of it pretty
tricky with lots of edge cases.
- There is a lot of code for synchronizing audio to MIDI. PortMidi
includes the ability to provide an external clock for synchronization.
Since audio clocks and system clocks are not synchronized (your audio
sample clock runs off a hardware crystal oscillator that's separate from
whatever the OS uses for time), I use PortAudio's sample count as a time
reference for MIDI. This is somewhat tricky because audio might not be
running, audio starts and stops, and the OS may not give PortAudio all the
info it needs to compute the current sample position accurately.
Nevertheless, I think the implementation is pretty good. What happens
between Audacity and PortMidi is Audacity generates timestamped MIDI
messages and delivers them to PortMidi more-or-less at the same time any
corresponding audio samples are delivered to PortAudio. PortMidi then (1)
translates the timestamp to system time (which is just adding an offset),
(2) adds a "latency" offset -- where latency is set to match the audio
latency of the system, (3) PortMidi then uses the MIDI device driver to get
the best timing accuracy possible. The goal is for MIDI messages to be
finally delivered just as corresponding audio samples are hitting the DAC.
Let me know if you have questions - this has been "swapped out" quite
awhile, but it would be cool to get it in production.
-Roger
Fortunately, the actual audio portion seems to mostly work (although it's

not completely stable and hangs sometimes) without changes; it's mainly the
UI that is broken currently. However, this information's certainly useful,
and should help with fixing the hanging (hopefully). Regarding
synchronization: one thing that doesn't seem to work is time tracks; note
tracks just don't seem to use them. But it seems to mostly stay in sync
otherwise (I haven't tested it too extensively, though).

--Poke

James Crook

2017-02-12 11:48:46 UTC

Permalink

Post by Pokechu22
Also regarding the mixer, what's the point of the MusicalInstrument type
(and related code)? I can't see where that shows up on the actual program,
but it sounds vaguely midi-related (yet, it isn't part of the midi code).

That's entirely about bitmaps that show on the Mixerboard.
http://manual.audacityteam.org/man/view_menu.html#Mixer_Board...

the bitmaps of the guitars, trumpet, keyboard, percussion

--James.

Pokechu22

2017-02-22 02:29:39 UTC

Permalink

Hey everyone,

I've gotten it mostly written at this point. The code is on the
fix-midi-output branch on my fork
(https://github.com/Pokechu22/audacity/tree/fix-midi-output) and you
can get a compare view at
https://github.com/audacity/audacity/compare/master...Pokechu22:fix-midi-output?expand=1.
(I haven't submitted a pull request yet).

These changes cover more or less only the UI related to midi playback;
they make no changes to the actual playback code other than fixing a
few compile warnings and errors. However, I think the UI changes are
fairly elegant and are beneficial even when EXPERIMENTAL_MIDI_OUT is
not enabled.

The mixer also supports note tracks, allowing changing the velocity
(instead of the gain as normally would be done). Most of the
left/right issues you previously mentioned were not applicable; they
were only due to weird naming due to the way EXPERIMENTAL_MIDI_OUT was
previously done with the mixer. I split it into 2 separate
MixerTrackCluster implementations, so while there still is both a
mTrack and an mLeftTrack/mRightTrack, mLeftTrack/mRightTrack are only
applicable to wave tracks.

I've also split the code for the midi channel toggle (the one seen on
http://manual.audacityteam.org/man/note_tracks.html) into its own
define (EXPERIMENTAL_MIDI_CONTROLS) so that it can be enabled without
midi output being enabled. It's fairly stable (and in fact has been
clickable in current releases even though it doesn't render) and
probably should be enabled in all builds, even ones without
EXPERIMENTAL_MIDI_OUT.

There's still a few things that I know are broken, primarily time
tracks. The actual midi output code in AudioIO is fairly complicated,
so I'm not completely sure whether I'd be able to handle that
immediately (I'm not completely comfortable with C++). Should this
first part be sent as a PR for testing so that other similar bugs can
be found?

--Poke

James Crook

2017-02-22 17:37:21 UTC

Permalink

This sounds good.

I haven't had time to try it out yet.
I think we should at the very least, during 2.2.0 development, have an
experimental build with your changes in, encourage users to try it, and
get feedback.

We've been talking on wiki about using experimental builds that are
believed to be suitable for production work to grow our team of engaged
users who give feedback on work in progress.
http://wiki.audacityteam.org/wiki/New_Builds_Team

--James.

Post by Pokechu22
Hey everyone,
I've gotten it mostly written at this point. The code is on the
fix-midi-output branch on my fork
(https://github.com/Pokechu22/audacity/tree/fix-midi-output) and you
can get a compare view at
https://github.com/audacity/audacity/compare/master...Pokechu22:fix-midi-output?expand=1.
(I haven't submitted a pull request yet).
These changes cover more or less only the UI related to midi playback;
they make no changes to the actual playback code other than fixing a
few compile warnings and errors. However, I think the UI changes are
fairly elegant and are beneficial even when EXPERIMENTAL_MIDI_OUT is
not enabled.
The mixer also supports note tracks, allowing changing the velocity
(instead of the gain as normally would be done). Most of the
left/right issues you previously mentioned were not applicable; they
were only due to weird naming due to the way EXPERIMENTAL_MIDI_OUT was
previously done with the mixer. I split it into 2 separate
MixerTrackCluster implementations, so while there still is both a
mTrack and an mLeftTrack/mRightTrack, mLeftTrack/mRightTrack are only
applicable to wave tracks.
I've also split the code for the midi channel toggle (the one seen on
http://manual.audacityteam.org/man/note_tracks.html) into its own
define (EXPERIMENTAL_MIDI_CONTROLS) so that it can be enabled without
midi output being enabled. It's fairly stable (and in fact has been
clickable in current releases even though it doesn't render) and
probably should be enabled in all builds, even ones without
EXPERIMENTAL_MIDI_OUT.
There's still a few things that I know are broken, primarily time
tracks. The actual midi output code in AudioIO is fairly complicated,
so I'm not completely sure whether I'd be able to handle that
immediately (I'm not completely comfortable with C++). Should this
first part be sent as a PR for testing so that other similar bugs can
be found?
--Poke
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel

Pokechu22

2017-03-21 04:51:56 UTC

Permalink

I've rebased my branch to include the latest changes. I've also made
a few other tweaks:

* Time tracks are now used with midi playback. This works in most
cases, but there's a very weird bug where when you _only_ have note
tracks and no audio tracks, the cursor desyncs (it goes at 1 over the
expected speed, so for 200% on the time track, the cursor visually
moves at 50% although playback occurs at 200%). This doesn't happen
if there's an audio track present, even if it's empty. I'm currently
not sure why that's happening; probably something simple though.
* Compiling with USE_MIDI disabled now works (it's broken in master
and has been for a while)
* Separate defines for the midi channel toggle buttons and midi
output, so that one can be enabled without the other (includes
handling positioning)
* Right-clicking the mute/solo button on note tracks no longer breaks things

One thing that isn't quite finished: scrubbing. It just doesn't play
notes most of the time, and isn't enabled unless there's also an audio
track. I'm not completely sure how scrubbing works; it may be
possible for me to figure it out.

As with before, my development branch is at
https://github.com/pokechu22/audacity/tree/fix-midi-output.

--Poke

Steve the Fiddle

2017-03-21 12:43:33 UTC

Permalink

I tried building this on Linux (Ubuntu 16.04 64-bit) but unfortunately
didn't get very far. I'm hitting loads of problems with PortMidi. I
couldn't get Audacity to build with the system library, and I couldn't
get the local library to build.

Steve

Post by Pokechu22
I've rebased my branch to include the latest changes. I've also made
* Time tracks are now used with midi playback. This works in most
cases, but there's a very weird bug where when you _only_ have note
tracks and no audio tracks, the cursor desyncs (it goes at 1 over the
expected speed, so for 200% on the time track, the cursor visually
moves at 50% although playback occurs at 200%). This doesn't happen
if there's an audio track present, even if it's empty. I'm currently
not sure why that's happening; probably something simple though.
* Compiling with USE_MIDI disabled now works (it's broken in master
and has been for a while)
* Separate defines for the midi channel toggle buttons and midi
output, so that one can be enabled without the other (includes
handling positioning)
* Right-clicking the mute/solo button on note tracks no longer breaks things
One thing that isn't quite finished: scrubbing. It just doesn't play
notes most of the time, and isn't enabled unless there's also an audio
track. I'm not completely sure how scrubbing works; it may be
possible for me to figure it out.
As with before, my development branch is at
https://github.com/pokechu22/audacity/tree/fix-midi-output.
--Poke
------------------------------------------------------------------------------

Paul Licameli

2017-03-21 13:21:36 UTC

Permalink

I'm the one who knows scrubbing best.

I made changes earlier so that EXPERIMENTAL_MIDI_OUT would compile again,
but it did not link, at least on macOS. Have you done anything to fix that?

One thing I want to accomplish this release is the track panel refactoring,
which is a major reorganization, and if one of us pushes first, the other
will have some difficulties merging, but I hope not too great.

PRL

Post by Steve the Fiddle
I tried building this on Linux (Ubuntu 16.04 64-bit) but unfortunately
didn't get very far. I'm hitting loads of problems with PortMidi. I
couldn't get Audacity to build with the system library, and I couldn't
get the local library to build.
Steve

things

Post by Pokechu22
One thing that isn't quite finished: scrubbing. It just doesn't play
notes most of the time, and isn't enabled unless there's also an audio
track. I'm not completely sure how scrubbing works; it may be
possible for me to figure it out.
As with before, my development branch is at
https://github.com/pokechu22/audacity/tree/fix-midi-output.
--Poke
------------------------------------------------------------

------------------
------------------------------------------------------------
------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel

Pokechu22

2017-03-21 15:36:01 UTC

Permalink

I've run into that too, when trying to compile on my Pi. I think that
the buildscript may be set up incorrectly, but I don't know for sure.
I managed to get it to link somehow, once, but it's not reliable.
I don't know anything about makefiles, so I can't fix it right now.

I can't find any reference to 'PortMidi' within any of the build scripts,
which seems like a bad sign (maybe it's just not included at all?)

It does work on Windows (using Visual Studio), though.

Post by Steve the Fiddle
I made changes earlier so that EXPERIMENTAL_MIDI_OUT would compile again,
but it did not link, at least on macOS. Have you done anything to fix that?
One thing I want to accomplish this release is the track panel refactoring,
which is a major reorganization, and if one of us pushes first, the other
will have some difficulties merging, but I hope not too great.
PRL

The linking failure was mostly due to EXPERIMENTAL_MIDI_OUT using the older
GainSlider(int) method (back when an array of sliders was allocated); since
that method no longer exists, it fails (but the method was still declared
when EXPERIMENTAL_MIDI_OUT was defined). I've fixed that by creating a
dedicated velocity slider that works the same way the gain slider works.
That also gets rid of the need to change the style of a slider, and generally
makes things a lot less messy.

There are still link errors related to libraries (which don't happen on
Windows, and I'm not sure how to fix), but none on the actual code within
Audacity.

If you're going to refactor further, that's fine; however, I've been writing
my code in the same style as the current track panel code, so it might make
sense to refactor my code too (though it depends on how the refactoring will
be done). Conflicts shouldn't be too much of an issue, though (I can resolve
them if needed - some of the existing changes actually caused conflicts but
they weren't too hard to fix).

Paul Licameli

2017-03-21 15:49:02 UTC

Permalink

Post by Pokechu22

I've run into that too, when trying to compile on my Pi. I think that
the buildscript may be set up incorrectly, but I don't know for sure.
I managed to get it to link somehow, once, but it's not reliable.
I don't know anything about makefiles, so I can't fix it right now.
I can't find any reference to 'PortMidi' within any of the build scripts,
which seems like a bad sign (maybe it's just not included at all?)
It does work on Windows (using Visual Studio), though.

Post by Steve the Fiddle
I made changes earlier so that EXPERIMENTAL_MIDI_OUT would compile again,
but it did not link, at least on macOS. Have you done anything to fix

that?

Post by Steve the Fiddle
One thing I want to accomplish this release is the track panel

refactoring,

Post by Steve the Fiddle
which is a major reorganization, and if one of us pushes first, the other
will have some difficulties merging, but I hope not too great.
PRL

The linking failure was mostly due to EXPERIMENTAL_MIDI_OUT using the older
GainSlider(int) method (back when an array of sliders was allocated); since
that method no longer exists, it fails (but the method was still declared
when EXPERIMENTAL_MIDI_OUT was defined). I've fixed that by creating a
dedicated velocity slider that works the same way the gain slider works.
That also gets rid of the need to change the style of a slider, and generally
makes things a lot less messy.
There are still link errors related to libraries (which don't happen on
Windows, and I'm not sure how to fix), but none on the actual code within
Audacity.
If you're going to refactor further, that's fine; however, I've been writing
my code in the same style as the current track panel code, so it might make
sense to refactor my code too (though it depends on how the refactoring will
be done). Conflicts shouldn't be too much of an issue, though (I can resolve
them if needed - some of the existing changes actually caused conflicts but
they weren't too hard to fix).

TrackPanel.cpp is just to darn big and does too many things.

My reorganization will move many things out into smaller files, and put
them behind virtual function interfaces.

You can look at my branch track-panel-refactor to see what the end result
will be like.

My more immediate priority is the another big project of adding exception
safety for errors reading and writing block files.

PRL

Post by Pokechu22
------------------------------------------------------------
------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel

Pokechu22

2017-03-21 20:19:17 UTC