gracefull handle Midi overflow

[daschuer]

This is a simple fix to handle midi overflows gracefully.
Details see: https://bugs.launchpad.net/mixxx/+bug/1527410

This fixes the issue that the midi fix stops after an overflow on nearly all platforms.
Unfortunately in Ubuntu the thing becomes worse due to and exit call.
This is why I change the required potmidi version to > the problematic one.

https://bugs.launchpad.net/mixxx/+bug/1527888
https://bugs.launchpad.net/mixxx/+bug/1097286

IMHO This is merge-able once we have a portmidi version for Ubnutu in our ppa

[rryan]

Hello! This branch is against the 1.12 branch, which is now closed to any changes except bugfixes. Please re-open your PR against master if this PR is still relevant.

[daschuer]

This is a bug-fix ;-)

[rryan]

The code change looks good to me though I have the same concern about the Debian package change. Instead, let's publish a portmidi package on our PPA compiled in release mode.

[rryan]

My PPA has a portmidi build with PM_CHECK_ERRORS disabled.
https://launchpad.net/~rryan/+archive/ubuntu/ppa/+packages
If someone could test, that would be great. If it works I can push it to our main PPAs.

[daschuer]

The package works for me on Precise. However I have tested it without a real controller.

[rryan]

@daschuer -- could you revert the control file changes?

[daschuer]

Yes, I can do it, but I want just verify that it is clear for you what it means:
After merging this and without a potmidi update, a Midi overflow and any other midi error will crash Mixxx.
I am not feeling comfortable to allow this. Do you?

[rryan]

I would rather the user be able to install Mixxx than to add a cryptic package requirement. Based on survey results, 70% of users don't have a MIDI controller. My bet is that most users who hit the dependency error when they download the .deb will give up.

You're probably right that this is not worth doing in 2.0.1 though. Maybe some other day when the Ubuntu situation is better.

[daschuer]

Ok I will remove the dependency tonight.

We should remove the direct *.deb download link from
http://www.mixxx.org/download/
Or provide libpormidi and libtag there as well.

[daschuer]

The situation does not change between 2.0.1 or 2.1.0 if we assume that every user installs our libportmidi this fix can go to both. If not, we need to force it by the dependency.

[daschuer]

By the way: done ;-)

[Be-ing]

This pull request has not been updated in years so I am closing it to reduce the clutter of open PRs. IMO if this requires changes to a dependency then we should not merge it until those changes are merged upstream.

[daschuer]

This one is waiting for the Ubuntu situation to go better.
it is fixed from libportmidi 1:217-6

Trusty: 1:200-0ubuntu3
Xenial: 1:200-0ubuntu3
Artful: 1:217-6
Bionic: 1:217-6

Maybe there is a way to detect the version at compile time.

[github-actions]

This PR is marked as stale because it has been open 90 days with no activity.

[Holzhaus]

This one is waiting for the Ubuntu situation to go better.
it is fixed from libportmidi 1:217-6

Trusty: 1:200-0ubuntu3
Xenial: 1:200-0ubuntu3
Artful: 1:217-6
Bionic: 1:217-6

@daschuer So it's ready to merge for 2.3? We drop support for older distros there.

[daschuer]

Yes finally. Thank you for the reminder.

[daschuer]

Done.

[daschuer]

This is a valid bug-fix. Who likes to have a look?

[Be-ing]

I'm not comfortable rushing this change into an almost ready to be released branch.

[daschuer]

I'm not comfortable rushing this change into an almost ready to be released branch.

Why are you at this opinion? What is the risky bit here.

The use of poll is here obviously redundant leading to a deadlock in rare cases.
PM_Read contains the same code as PM_Poll
https://github.com/jwinarske/portmidi/blob/a588cf60f47cd6eeb8b1cca09cd9670702731ba3/pm_common/portmidi.c#L374

[Holzhaus]

@Be-ing if this fixes a known bug we should merge, unless you have evidence that this change might introduce other issues.

@daschuer Is there some way to test this? Code lgtm.

[daschuer]

You can slow down the polling rate and turn the jog wheel to produce an overflow.
Without the patch midi stops. With the patch it continues.

[daschuer]

@Holzhaus Did you find time to test this?

[Holzhaus]

Sorry, forgot about this. Thanks for the reminder. Works fine for me. I can't comment about the code though.

[Holzhaus]

@Be-ing merge?

[Be-ing]

I'm still nervous about merging this without a clear way to reproduce the issue and knowing that the MIDI I/O code otherwise has been working well for years. However maybe this addresses the sporadic reports of controllers randomly not working after a while on Windows? So if consensus is to merge this now, I'm not going to insist on holding it up.