search

notahat / midi_patchbay

Hook up MIDI software and hardware and pass MIDI data between them, applying assorted filters on the way.
228 stars 54 forks source link

modernization update #4

Open danomatika opened 4 years ago

danomatika commented 4 years ago

This PR a general modernization update for building the app for macOS 10.10+ as well as 64 bit builds for 10.15+.

In also integrates existing PRs #1 and #2 as well as a commit from one of the forks. I also took the liberty of updating the (meta) documentation to markdown.

I find this application extremely useful and would very much like to see an official release of this proposed 1.0.4 version as a 64 bit build. It's obvious the design is solid and the CoreMIDI API is stable as MIDI Patchbay has worked just fine over the last 10 years. Hopefully this afternoon of work will ensure that it does for the next 10. :)

EDIT: 64 bit version 1.04 test builds are here: http://docs.danomatika.com/releases/midipatchbay/

NOTE: For attribution, I should probably listed as "Dan Wilcox (ZKM | Hertz-lab)" to include the fact this work was done within my normal job.

notahat commented 4 years ago

@danomatika You're a champion! I'll try to have a look this weekend and get an official release out. I'll add you to the credits and give you a mention on the website too.

danomatika commented 4 years ago

Thanks. It's mainly a community effort I pulled together: see the attribution in the proposed 1.0.4 version at the end of the readme.

There are still two points which are in question:

  1. I noticed the UI in the repo is slightly different from the last release (1.0.3). I find the release version to be a better layout, but I wasn't sure enough about the design decisions involved to change it directly.
    • placement of the Input & Outputs
    • placement of Add Patch button and additional Remove Patch button
    • 1.0.3 opens with Notes tab selected by default instead of Channels tab

In the repo:

Screen Shot 2019-12-06 at 11 19 08 AM

Version 1.0.3:

Screen Shot 2019-12-06 at 11 19 22 AM
  1. There is still a deprecation warning for the custom TableView Enter key handling which was non-trivial to change.

I will make proposed implications for both of these issues in feature branches and post back.

danomatika commented 4 years ago

Oh, and ARC modernization is needed as well.

danomatika commented 4 years ago

Recent work:

EDIT: That should it for now. I think everything is covered, unless bugs or more inconsistencies with the 1.0.3 release version are found.

danomatika commented 4 years ago

Also, there is a new test build with the UI updates.

notahat commented 4 years ago

Oh, and ARC modernization is needed as well.

Yeah, I'm going through the changes, and I'm suspicious of all the memory management stuff. The fun is that CoreMIDI doesn't correctly follow Apple's own memory management rules, so I'm not sure how it's gonna interact with ARC. Gonna have to do some more reading and testing.

XENONChromatic commented 4 years ago

I am happy to continue testing as needed. The current version (RC2 I guess it'd be called?) seems to run fine, and I didnt see any noticeable performance issues in terms of memory/CPU usage, although I also wasnt looking for such things.

danomatika commented 4 years ago

It’s all in a single commit, so it should be straightforward to review. I have a couple projects using CoreMIDI so I know what you mean. Most of what I did was to add bridging casts for strings and you were already deep-copying those coming from and owned by CoreMIDI, as far as I can tell. I can double check as well.

enohp ym morf tnes

Dan Wilcox danomatika.com robotcowboy.com

On Dec 8, 2019, at 12:14 AM, Pete Yandell notifications@github.com wrote:

Oh, and ARC modernization is needed as well.

Yeah, I'm going through the changes, and I'm suspicious of all the memory management stuff. The fun is that CoreMIDI doesn't correctly follow Apple's own memory management rules, so I'm not sure how it's gonna interact with ARC. Gonna have to do some more reading and testing.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

danomatika commented 4 years ago

@notahat I'm happy about #6. This PR is definitely more quick triage as I went quickly without a deep understanding of the code. If it's helpful in the end, great.

EricGeorge commented 4 years ago

@danomatika - so glad you did this. I too was slowing trying to bring this up to date and super glad to see the activity here. I did some quick testing on the test build and it seems to work well on 10.15.2. Happy to continue testing as needed until merged.

mmontag commented 3 years ago

Can this be merged? Or is it now obsolete?