You could try detecting an all-channels-off state and use that rather than
the no-write condition. As opl emulation is rather demanding it's something
definitely to keep.

I figured it was important to keep, or it wouldn't be there, so I made this post about increasing the awareness of the code, not eliminating it. I'm reluctant to add to the fmopl.c code because it has a lot of overhead as it is, and it has separate methods for different OPL chips. My thought is to make something that is generic to all OPL types, adds as little code and processing as possible, and sticks to areas of the program that are less tricky to deal with.

I came up with this modification to the code block shown in my initial post:

1   if ((PIC_Ticks-opl.last_used)>30000) {
2      Bit16s sound=0;
3      for (i=0;i<len;i++) sound|=((Bit16s*)MixTemp)[i];
4      if (sound==0) {
5         opl.chan->Enable(false);
6         opl.active=false;
7      } else opl.last_used=PIC_Ticks;
8   }

Not the most elegant solution, but it is a small change that adds little extra processing. When the old logic would disable output after 30 seconds of no port writes, this goes one step further and inspects the output buffer generated by the callback and only disables if there is complete silence; however, if there is still some sound going on, the timeout is extended by another 30 seconds.

My tests so far show it to work as intended, but I will continue to test more games with it. Where there might be problems is if 0 samples aren't generated when a game suspends OPL output or exits out to DOS, but I have yet to see that occur.

There are some cases where zero samples are not generated, instead it's a non-zero sample that's repeated; which is also a "silence" condition. Changing the loop to look for a repeated sample in the buffer covers these cases:

1   if ((PIC_Ticks-opl.last_used)>30000) {
2      Bit16s sample=((Bit16s*)MixTemp)[0];
3      for (i=1;i<len;i++) if (((Bit16s*)MixTemp)[i]!=sample) break;
4      if (i==len) {
5         opl.chan->Enable(false);
6         opl.active=false;
7      } else opl.last_used=PIC_Ticks;
8   }

While it does work, it's still a hack. I'll next try logging the OPL register writes to see if there is a reliable pattern that can be used. Some kind of "all off" command would be ideal, but I guess that would already be used to disable output if it existed.

To implement wd's idea about channel states, I did some research on Adlib registers. The "Key On" status (bit 5 of registers 0xB0 - 0xB8) works for this purpose, because the channel is voiced when it is set, and silent when it is clear. There are 2 sets of registers to check because of dual OPL mode:

1   if ((PIC_Ticks-opl.last_used)>30000) {
2      /* Disable output if all channels are silenced */
3      for (i=0xb0;i<0xb9;i++) if (opl.raw.regs[0][i]&0x20||opl.raw.regs[1][i]&0x20) break;
4      if (i==0xb9) {
5         opl.chan->Enable(false);
6         opl.active=false;
7      } else opl.last_used=PIC_Ticks;
8   }

This approach seems more conventional than analyzing the output buffer. It would be possible to use the register test on its own, rather than an additional test inside the 30 second timeout; but my testing showed that doing so creates a considerable amount of enabling and disabling, which can create many "pops" in the sound output if the samples in the buffer are not zeros (such as that repeated non-zero sample mentioned previously).

I've tested it with many games, including the constant sfx cases, and it works well. A clearer coding style using an extra boolean var might be preferred for inclusion in CVS, but the basic concept would be the same.

and silent when it is clear

Well this is wrong, keyoff triggers the release which eventually results in an
off state if the amplitude multiplier is low enough. I've got to check if the
release always decreases the amplitud, as then it would be ok to turn the
speaker off after some time.

Well, the online docs I looked at described it as an on/off kind of thing. But as you say, perhaps the 30 second timeout will ensure that output has tapered off in most (normal) cases.

However, the proposed change doesn't look for channels that are OFF; it persists the enabled state of output if it finds any channels are ON (despite the no-write timeout), and the ON state should be a reliable signal, right?

ripsaw8080 wrote:

Well, the online docs I looked at described it as an on/off kind of thing. But as you say, perhaps the 30 second timeout will ensure that output has tapered off in most (normal) cases.

Well, setting a decay rate of 0 (slowest) and a release rate of 0 (also slowest) should do the trick to create a maximum delay after the note off. Also, check reg 0xBD: when in rhythmic mode, bits 0-4 determine on/off of the drums, instead of the key-on for channels 6-8. Although not typical for music, a sound effect may use the drums with a long sustain.

JAL

setting a decay rate of 0 (slowest) and a release rate of 0 (also slowest) should do the trick to create a maximum delay after the note off

Some of the parameters are special, and i think the 0-rates are effectively
non decreasing, that's why i mentioned it (then even 30 sec past-delay
does not work, just as it currently fails to work).

the online docs I looked at described it as an on/off kind of thing.

It is called OFF, yes, but as noted it does not have an instant effect as the
channels are seen as instruments (with an outfading component, the release).

t is called OFF, yes, but as noted it does not have an instant effect as the
channels are seen as instruments (with an outfading component, the release).

I don't know if you missed my edit or not, but again, OFF is not important to the proposed change. A channel that is ON is being used to persist the enabled state of output where it would have been disabled before. For channels that are "off", the timeout would already be cutting off output prematurely in these theoretical cases you're bringing up; and I don't recall ever encountering such a case, anyway. You're increasing the scope of the problem.

I was just referring to your statement

and silent when it is clear

which, whatever docs this is from, is plain wrong for the mentioned reasons.

I am aware that your conditions do not rely on that.

"Programming the AdLib/Sound Blaster FM Music Chips" by Jefferey S. Lee, which has become fairly ubiquitous on the internet judging by how often it turns up in Google searches on the subject. I suspect it's just a poor choice of words, though.

http://www.shipbrook.com/jeff/sb.html

Can you recommend a more thorough document?

Check out the ymf262 datasheet (google should turn up some pdfs),
there's some nice picture about the whole envelope generator that
depicts the effect of the keyoff (switching to release mode).

ripsaw8080 wrote:

"Programming the AdLib/Sound Blaster FM Music Chips" by Jefferey S. Lee

That is about the only document that seems freely available, though it is rather concise and does not describe what exactly happens when.

JAL

wd wrote:

Check out the ymf262 datasheet (google should turn up some pdfs), there's some nice picture about the whole envelope generator that
depicts the effect of the keyoff (switching to release mode).

Ah yes, I have that one already, I forgot. And the YMF3812 as well, which has a slightly different but comparable one. It mentions explicitly that percussion tones do not have a sustain, but the OPL3 one doesn't mention that, it seems.

JAL

ymf3812 is opl2, also even older opl chip manuals are handy as they are
by times more explicit and have nicer drawings.
Other resources that i found quite handy (note that they contain errors):
http://www.4front-tech.com/dmguide/dmfm.html
http://www.fit.vutbr.cz/~arnost/opl/opl3.html

I was reminded of this issue from a recent thread; and since the OPL code has changed quite a bit since 0.72, I thought I'd make an updated patch for current source (also works with 0.73 release source). The idea remains the same: only shut off the FM sound generation after 30 seconds if none of the channels has the KeyOn status bit set. Here is a list of games that benefit from the patch. There are almost certainly others, but I've confirmed the issue in these:

Falcon 3.0 (engine sound)
Jetfighter II (engine sound)
Lightspeed/Hyperspeed (engine sound)
Space Quest 3 (conveyor belt in garbage ship)
Desert Strike/Jungle Strike (helicopter blades)

Where does the 30 seconds value come from?

Only the devs know for sure, but I think the delay is to keep it from excessively turning on and off, which would be inefficient and there is sometimes a popping sound that occurs at on/off. 30 seconds was probably chosen for being long enough, but not too long. The PC speaker emulation uses a 10 second turn-off delay. The comment in the code says "Disable the sound generation after 30 seconds of silence", however the issue with FM sound is that port inactivity does not necessarily equate to silence, because some games set up constant sfx where port writes are not needed to sustain the sound.

added it for the time being. thanks.