Viewing Issue Advanced Details
[ Jump to Notes ]
ID Category [?] Severity [?] Reproducibility Date Submitted Last Update
02934 Sound Minor Always Feb 20, 2009, 04:38 Mar 8, 2009, 17:03
Tester chowell View Status Public Platform MAME (Official Binary)
Assigned To Resolution Open OS
Status [?] Acknowledged Driver
Version 0.129u4 Fixed in Version Build Normal
Fixed in Git Commit Github Pull Request #
Summary 02934: DISCRETE: Output voltage of DISC_MIXER_IS_OP_AMP is not offset by vRef if vRef is nonzero.
Description For the DISC_MIXER_IS_OP_AMP (no rI) version of DISCRETE_MIXER, if vRef is not zero, the output voltage will not be offset by vRef as it should.

For reference, the general circuit diagram of a DISC_MIXER_IS_OP_AMP (no rI) is:
                                               cF
                                          .----||---.
                                          |         |
        rNode[0]    r[0]   c[0]           |    rF   |
   IN0 >--zzzz------zzzz----||---.        +---ZZZZ--+
                                 |        |         |
        rNode[1]    r[1]   c[1]  |        |  |\     |
   IN1 >--zzzz------zzzz----||---+--------+  | \    |
    .      .         .      .    |        '--|- \   |  cAmp
    .      .         .      .    |           |   >--+---||-----> Netlist Node
    .      .         .      .    |        .--|+ /
    .   rNode[7]    r[7]   c[7]  |        |  | /
   IN7 >--zzzz------zzzz----||---'        |  |/
                                          |
  vRef >----------------------------------'
In DISCRETE_STEP(dst_mixer) within emu/sound/disc_mth.c, the op-amp's output voltage for a DISC_MIXER_IS_OP_AMP is computed as follows (line 1138; all line numbers are for 129u4):
          v = i * ((type == DISC_MIXER_IS_OP_AMP) ? info->rF : r_total);
This determines the op-amp output voltage v using the fact that the feedback current across rF must equal the total net reverse current into the input lines. The only problem is that it assumes that vRef = 0 V. Thus v_ref should be added to v for the DISC_MIXER_IS_OP_AMP case.

Likewise, line 1152:
              v = context->v_cap_f;
sets the op-amp output voltage v when the op-amp has a capacitor cF, based on the fact that v must be the same as the voltage across cF. Again this assumes vRef = 0 V, so v_ref needs to be added here as well.

Assuming these changes are made, the comment on line 1013 should also be updated to match.

I'm not sure how serious this discrepancy is, since the result is only a DC offset, so normally there would be no audible effect. But you never know when somebody might use a DISC_MIXER_IS_OP_AMP in a way where the DC level would matter, such as if it were passed to the input of some other circuit. Anyway, since the output voltage appears to be correctly offset for DISC_MIXER_IS_OP_AMP_WITH_RI, it seems that the module should at least be consistent.

For reference, the following mixer descriptions use a DISC_MIXER_IS_OP_AMP without rI with a nonzero vRef: atarifb_mixer, abaseb_mixer, bsktball_mixer, firetrk_mixer, bosco_filter_mixer, galaga_final_mixer. These are in mame/audio/{atarifb.c,bsktball.c,firetrk.c,galaga.c}.
Steps To Reproduce
Additional Information
Github Commit
Flags
Regression Version
Affected Sets / Systems DISCRETE
Attached Files
  
Relationships
There are no relationship linked to this issue.
Notes
5
User avatar
No.03762
chowell
Developer
Feb 20, 2009, 04:43
 Grr, typo in summary. Fixed.

I'd also like to mention that I may have found a few other bugs in DISCRETE_MIXER, but I'm not sure about those yet. The code is rather complex to analyze because of all the different possibilities, and also because it handles two or three different types of circuits in significantly different ways.
User avatar
No.03769
couriersud
Developer
Feb 21, 2009, 14:54
 Derrick would be more appropriate to fix this case. I am a bit reluctant to change something right now because this may have an effect on the output levels of a lot of games, namely those without a Camp and further high-pass filtering.
User avatar
No.03771
chowell
Developer
Feb 21, 2009, 22:18
 I don't mind if you don't want to fix this right away. I understand that any changes to DISCRETE_MIXER would require checking and possibly modifying a bunch of drivers to ensure they continue working.

There appears to be one other complication. The output of DISCRETE_MIXER is always multiplied by the gain value in the mixer description. This gain seems to be a simple level boost for convenience, rather than representing an actual gain from the mixing circuit.

If vRef was simply added to the output to give the correct output voltage level, this DC offset would also be multiplied by the gain value, which I imagine would be undesired. So that problem would have to be solved.

It seems to me that the gain boost is not really part of the mixer circuit emulation and should be handled elsewhere, such as DISCRETE_OUTPUT, which also has a gain parameter. But changing this would obviously require modifying a lot of drivers, so I can understand if you don't want to do it right away. And maybe you want to remove any DC offset from the output of DISCRETE_MIXER--perhaps it's currently the simplest way to do that before generating the audio sample data. If so, it would be nice if that were made explicit, and perhaps configurable.
User avatar
No.03772
couriersud
Developer
Feb 21, 2009, 22:38
 That's what cAmp is for. If specified, the module will assume a 50K load (amplifier stage) after it and do high-pass filtering.
User avatar
No.03921
derrickr
Developer
Mar 8, 2009, 17:01
edited on: Mar 8, 2009, 17:04
 Yes the gain boost is just for convenience. I will probably remove it at some point in time.

The thing to remember, though it's not a good excuse, is that the discrete system started off as being only a generic sound system. The intent was never to simulate at the component level. But over time I found it easier to just write mini-simulations for each module using real R/C values. This saved the time of analyzing the circuit and creating a generic waveform.

So as things progressed, generic and component value code was mixed together. Now most of the generic waveform modules are no longer used. Things are modified on an ongoing basis when the need is real.

I try to make the modules as universal as possible, but there is always another case that crops up where the circuit is similar to the current module, but with a small extra twist added.

If you want to play around, I suggest you use the DISCRETE_CUSTOM module, copy the current code to it and see what happens. You can use the DISCRETE_WAVELOG to output waveforms using a gain of 1000 so the resulting wav file uses 1000 samples per volt and can be viewed in Adobe Audition or similar program and compared to a SPICE program. But remember that SPICE programs are not always accurate at a voltage level. They assume perfect voltage output of ICs, and not the real world levels.

As for your main concerns in the report, I will take a look when I get a chance. Thanks and keep up the well thought out reports and code analyzing.