Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Various reliability and correctness improvements to MIDI on Windows #7620

Merged
merged 8 commits into from Jul 4, 2019

Conversation

nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Jun 10, 2019

  • Improve use of locks in Win32 driver to prevent more race conditions
  • Share the standard SysEx message data between Win32 and DMusic drivers
  • Send the GM reset SysEx message before each song to ensure correct reset of device
  • Remove the manual controller resets as they are included in the GM reset

I probably should try to split this into multiple commits.

The last point is tested on a Roland and a Yamaha synth, both behave as expected.

Should fix #7438 (by resetting correctly between tracks)

@@ -35,14 +35,14 @@ static struct {
CRITICAL_SECTION lock; ///< synchronization for playback status fields

bool playing; ///< flag indicating that playback is active
bool do_start; ///< flag for starting playback of next_file at next opportunity
int do_start; ///< flag for starting playback of next_file at next opportunity
Copy link
Member

@LordAro LordAro Jun 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be an enum?

Copy link
Contributor Author

@nielsmh nielsmh Jul 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The members would be StartStep1, StartStep2, StartStep3, etc., I don't see a way to make a meaningful enum of this.

Ideally MidiFile would scan for various different init sequences in a file, and if a known sequence is found then leave it, otherwise prepend an init sequence (GM reset + reverb settings) to the data so the driver doesn't need to do anything difficult. I have more plans for improving music support (also past MIDI) and this idea is probably better left for then.

src/music/dmusic.cpp Show resolved Hide resolved
src/music/win32_m.cpp Outdated Show resolved Hide resolved
src/music/win32_m.cpp Outdated Show resolved Hide resolved
src/music/win32_m.cpp Outdated Show resolved Hide resolved
src/music/win32_m.cpp Outdated Show resolved Hide resolved
src/music/win32_m.cpp Outdated Show resolved Hide resolved
michicc
michicc approved these changes Jul 4, 2019
@nielsmh nielsmh merged commit a0c78c7 into OpenTTD:master Jul 4, 2019
8 checks passed
@nielsmh nielsmh deleted the musicfixes2 branch Jul 5, 2019
michicc pushed a commit to michicc/OpenTTD that referenced this issue Jul 7, 2019
@michicc michicc added the backport requested label Jul 7, 2019
michicc pushed a commit to michicc/OpenTTD that referenced this issue Jul 7, 2019
michicc pushed a commit to michicc/OpenTTD that referenced this issue Jul 7, 2019
michicc pushed a commit to michicc/OpenTTD that referenced this issue Jul 7, 2019
@michicc michicc added backported and removed backport requested labels Jul 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants