Wohlstand / libADLMIDI

A Software MIDI Synthesizer library with OPL3 (YMF262) emulator
GNU Lesser General Public License v3.0
174 stars 17 forks source link

CC#120 all sound off not implemented #48

Closed jpcima closed 6 years ago

jpcima commented 6 years ago

It does not handle this controller. It is supposed to mute a channel's sound output. In my own fmidi sequencer, I send CC120 to silence the audio when there is a change of track. TiMidity++ recognizes it.

Indeed code reveals it is not a handled cc number. There is however 123, a similar one. (all notes off). From this, I guess it's probably not a thing on my side to implement, but an omission of ADLMIDI.

cf. MIDIplay::realTime_Controller, adlmidi_midiplay.cpp

Wohlstand commented 6 years ago

I think, I need to add that, useful event that must be supported!

EDIT: Yeah, seems to implement I will need to do same as CC123 but also shut up all chip channels.

Wohlstand commented 6 years ago

Just now I have added that! :fox_face: The test MIDI file I made to test both CC123 and new CC120! сс120-123-test.mid.zip

jpcima commented 6 years ago

This is not right. Playing back a normal midi I can hear nothing except percussions now. I have pulled your commits into adljack. Until this, I just changed libADLMIDI to make cc120 do the same as 123.

For reference this is how I use the cc120 (which worked with timidity in alsa midi server). midi_reset I send this event sequence to reinitialize the instruments between tracks. In reality I don't take this event from the midi files.

Wohlstand commented 6 years ago

I have checked the behavior of those events on FluidSynth until I began to implement it. Anyway, can you send me the MIDI file to let me polish my stuff with it? The CC120 must shut up all playing notes completely without giving them to play release while CC123 just sends NoteOffs to all playing notes in a current channel.

EDIT: Mine a bit updated example where I have been added some notes following CC120 in the second channel: сс120-123-test.mid.zip

EDIT2: I have checked Timidity too, that works fine at me, or what do you mean "not right"? Can you illustrate the results?

jpcima commented 6 years ago

It's not a problem of one MIDI file, it's a problem of all of them. The only thing is that I work in realtime playback.

Before I pull the last commits => I have sound After => only percussion

And the stranger thing is, it does not even happen in relation with all sound off events. On a fresh instance of an ADL player, where cc120 is not sent, the sound channels still start muted.

jpcima commented 6 years ago

Here is a picture. Maybe it shows what I mean better.

screen2

jpcima commented 6 years ago

Ok here's breaking news. It must have to do with the new commits about emulation cores. Because without these but only the cc120 patch, the sound does play.

edit: and cc120 does its intended thing, I shall add

Wohlstand commented 6 years ago

Anyway, I checked out your adljack, and seems it works unstable, and plays few notes, I ran RtMidi thing I built from sources together with RtAudio. Even I playing MIDI file which has no any CC120 events. Also, I have reduced minimal CMake into 3.5 from 3.6 as I have 3.5 installed, and C++14 instead of C++17 as my compiler GCC 5.4.1 doesn't supports it as I can see. However, everything was built with no errors.

Wohlstand commented 6 years ago

P.S. I'll aply CC120 patch to OPNMIDI too as it's confirmed it working right.

jpcima commented 6 years ago

I checked out your adljack, and seems it works unstable

(ah! Maybe I know why, now you mention it. I have to test alsa on motherboard audio, because I'm on a firewire card)

Is the new NukedOPL 1.8 the origin of the sound problems?

Wohlstand commented 6 years ago

Oh, yeah, Nuked OPL3 1.8 had some sounding issue I have been resolved!

jpcima commented 6 years ago

I possibly fixed adlrt, please check. The default latency of rtaudio was just set way too low. I add the flag [-L latency-ms] to choose a latency value. default 20ms

Wohlstand commented 6 years ago

I hope, this thing seems useful, Ill try with bigger latency a bit later, for now I gonna sleep and I have powered down my PC.

Отправлено с Samsung Mobile

-------- Исходное сообщение -------- От: JP Cimalando notifications@github.com Дата:10.04.2018 5:08 (GMT+03:00) Кому: Wohlstand/libADLMIDI libADLMIDI@noreply.github.com Копия: Vitaly Novichkov admin@wohlnet.ru,Assign assign@noreply.github.com Тема: Re: [Wohlstand/libADLMIDI] CC#120 all sound off not implemented (#48)

I possibly fixed adlrt, please check. The default latency of rtaudio was just set way too low. I add the flag [-L latency-ms] to choose a latency value. default 20ms

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

jpcima commented 6 years ago

Ok thanks for all the things. Good night

jpcima commented 6 years ago

Just for the record so far here's what I know about this sound problem. If you can run adl-rt, and pull latest libADLMIDI, it's a decent chance you will experience what I speak about.

Wohlstand commented 6 years ago

@jpcima, Okay, just now I figured out for reason why that thing wasn't worked: https://github.com/Wohlstand/libADLMIDI/commit/ff0982dd98cd7a828730a3498e3e7b9cd1daf868 the mistake was so stupid :man_facepalming: OPLChipBase::generate() does replace of content of given buffer, and I needed to use OPLChipBase::generateAndMix() call which mixes a given buffer with new-generated data.

When I fixed that, now latest state playing fine at my machine, and with Latency 5 milliseconds that playing fine :smile:

P.S. @jpcima, THANKS for that thing! :+1:

jpcima commented 6 years ago

Oh thanks god for finding this!!!! I was going to look into it more after doing some details in the rt player.

Wohlstand commented 6 years ago

Okay, as the main topic of this issue resolved, I'll close it. For RTMIDI discussion let's create a new issue :wink:

Wohlstand commented 6 years ago

Discussion for RT-MIDI is moved to here: https://github.com/Wohlstand/libADLMIDI/issues/53