-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Adding sound support to Model Racing Dribbling driver (part 2) #12147
Conversation
Copying the conversation over. I am opening up the PR (I will try or open a new one and link to here) but please provide guidance on:
Exactly. That is why an OpAmp (idealized?) is a good approximation. JFET:
Nope, I can try. So can I get help and guidance? this is close to my skill border Using named objects: I don't understand what it means, a link with an example would suffice. Keeping things as is it's an option as well since it's only 9 sounds total. High pitch removal additional circuitry: let me know what you prefer, I can only comment that either we keep it (with a #define for now) or we add an OpAmp 1x amplifier to act as cut-off filter. My quick attempts at the latter haven't produced results as clean as the logic circuitry. Logs: I agree to disagree but I am totally fine in putting them back as they don't hurt performance anyway. Sorry if I closed comments that seemed to be just questions on why had I taken some choices, I believe the github threaded model doesn't help with this specific type of discussion.
One more thing: the STOP_PALLA (and PARATA I guess) require an higher resolution since the code switches the circuitry off too fast for the netlist code to realized that there was an event. They can be moved to a different netlist, but it won't help anyway since they require the LM339. |
Look at the implementation of the op-amp in the C++ code. Most of it’s just a netlist. It uses a voltage-controlled current source for the output, and some resistors, capacitors and diodes. You can build a simplified LM339 model in your netlist the same way. Use a similar arrangement of resistors for the input, and use a limited voltage-controlled current source for the output.
It isn’t though, because it will affect the waveforms of the function generators, and if you did it in real life it would burn out one of the JFETs. The primary issue is that the LM339 has a single-ended output and cannot source current. An op-amp, which can source current, will produce different waveforms and also likely burn out one of the JFETs rather quickly. From a quick look at the FET model:
They’re absolutely zero run time cost when they’re disabled. It’s never a bad thing to help the next person who comes along. |
src/mame/mr/dribling.cpp
Outdated
NETLIST_STREAM_OUTPUT(config, "snd_nl:cout0", 0, "OUTPUT").set_mult_offset(1.0, 0); | ||
|
||
|
||
} | ||
|
||
/************************************* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There’s still the diff noise here from adding an extra blank line inside the function and getting rid of one from the two blank lines separating functions.
Can you force an update of the sound simulation when the code changes the state of the signal? Is it possible to force the netlist sound device to update using |
If you are referring to |
Another option I've seen float around on forums. Spice has a library device called DIFFSCHMITT that does exactly this. (Or maybe simply map V+ to SCHMITT input, V- to SCHMITT GND) |
wrt
I will try and report back. |
I've tested the idea and seems very promising. Without fine tuning the parameters in the SCHMITT based comparator the diffs are looking promising. This is the STOP_PALLA netlist running under both models: Performance of the transistor model: If there is consensus I can start a new PR for adding the DIFF_SCHMITT_TRIGGER and close the chapter on LM339 emulation once I obtain almost identical output. |
src/mame/mr/dribling.cpp
Outdated
NETLIST_LOGIC_INPUT(config, m_i_pb.at(0), "I_PB0.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(1), "I_PB1.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(2), "I_PB2.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(3), "I_PB3.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(4), "I_PB4.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(5), "I_PB5.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(6), "I_PB6.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(7), "I_PB7.IN", 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You’re supposed to index array-like things like arrays – m_i.pp[0]
etc. Using at(...)
has additional overhead because it does a bounds check to throw an exception if you pass it a bad index, which you can tell won’t happen at compile time.
src/mame/mr/dribling.cpp
Outdated
NETLIST_LOGIC_INPUT(config, m_i_pb.at(0), "I_PB0.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(1), "I_PB1.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(2), "I_PB2.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(3), "I_PB3.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(4), "I_PB4.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(5), "I_PB5.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(6), "I_PB6.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_pb.at(7), "I_PB7.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_folla_b, "I_FOLLA_B.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_folla_m, "I_FOLLA_M.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_folla_a, "I_FOLLA_A.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_calcio_a, "I_CALCIO_A.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_fischio, "I_FISCHIO.IN",0); | ||
NETLIST_LOGIC_INPUT(config, m_i_calcio_b, "I_CALCIO_B.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_contrasto, "I_CONTRASTO.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_stop_palla,"I_STOP_PALLA.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_parata, "I_PARATA.IN", 0); | ||
NETLIST_LOGIC_INPUT(config, m_i_enable, "ENABLE_SOUND.IN",0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you’re going to tabulate these, please do it neatly so all the node names in the group line up properly. Otherwise just use one space after a comma. Right now there are various different numbers of spaces after commas, but the node names still don’t line up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they look better now.
It would be nice to have a datasheet for the JFETs. The schematic seems to say that the JFETs are 2N3812, but the only 2N3812 I know of is a matched pair of NPN transistors for differential amplifier input stages. I’m wondering if the schematic is wrong and the JFETs are actually some other type. Do you have access to a board? Can you check what type of JFETs are used? Alternatively, is there a parts list or something that can be cross-checked against the schematic? At any rate, the pinch-off voltage for these kinds of JFETs is usually around -3V, and given how they’re using it, the Vgs=0 current might be around 2mA at a guess given how they’re used. Are you connecting the MOSFETs the right way around? For the one in the function generator, the source and gate need to be connected to the LM339 output, and the drain needs to be connected to the timing capacitor. The diode across it should have the anode connected to the gate/source and the cathode connected to the drain. |
It’s more complex than necessary – you’d be using a model with hysteresis to simulate something without hysteresis. It would be better if you just wait for someone to help you with a simplified LM339 model. |
Given nobody actively works on this stuff and this is a major improvement over the current state as things stand, is there some acceptable imperfect version we can get in and IMPERFECT_SOUND? MAME's supposed to have the best version of what we know about the hardware, and this is significantly better than not having sound or using samples. |
Can we at least try to work through this first?
I also get the feeling that if this goes in as-is, people will do a media blitz over-selling it as “accurate” dribbling, but it will just sit in that state forever without being fixed properly. It’s feeling increasingly pushy. It’s only been open a few days. It seems I’m the only one actually looking at this, and I’m doing my best to explain things, but it’s now turning into a constant distraction making it hard to actually get other overdue stuff done. |
I am pretty sure it's a typo in the schematic as there is 2N3821 JFET. |
That doesn’t seem right – a 2N3821 is a VHF amplifier JFET in a TO72 metal can package (i.e. for the RF or IF section of VHF radio receivers). I wouldn’t expect them to use something that expensive for a simple audio function generator. |
Considering that propagation delay are ignored I am having the exact opposite problem, i.e. the oscillation stops a bit too late because the last few oscillations shouldn't happen. I am working around it by using a minimum voltage check that reflects the real hardware cut-offs and the result is great. |
Vas, as much as I want to get this merged in, I agree with you as it seems we are very close. OTOH if you trust me to push this to completion, we can merge after:
What do you think? |
…ic but commented behind a #define
As rb6502 said:
Ok, the simplified ideal comparator works great! (kudos to Vas for providing the Schmitt trigger code; and me for adapting it). Below is the output of the STOP_PALLA sound and looks incredibly close to the expected, no work arounds required. Considering that:
Can we converge on fixing the remaining code review issues - if any - and get it merged? |
src/mame/mr/nl_dribling.cpp
Outdated
// 3rd order Butterworth filter approximation. | ||
SUBMODEL(output_filter, OUTPUT_FILTER) | ||
SUBMODEL(output_filter, OUTPUT_FILTER_2) | ||
SUBMODEL(output_filter, OUTPUT_FILTER_3) | ||
NET_C(OUTPUT_FILTER.INPUT, Q_OUT.C) | ||
NET_C(OUTPUT_FILTER.GND,OUTPUT_FILTER_2.GND, OUTPUT_FILTER_3.GND, GND) | ||
NET_C(OUTPUT_FILTER.OUTPUT, OUTPUT_FILTER_2.INPUT) | ||
NET_C(OUTPUT_FILTER_2.OUTPUT, OUTPUT_FILTER_3.INPUT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
24kHz corner frequency is too high. For a 48kHz sample rate, the Nyquist frequency is 24kHz. You want anything above this to be suppressed significantly. This isn’t going to happen if you have the corner frequency right at the Nyquist frequency. The TDA2003 has a cutoff frequency of 15kHz – that might be a better choice.
Also, this is an over-complicated way of doing this. You don’t need to cascade three first-order filters. It’s more efficient to do it in a single stage with the Sallen-Key arrangement.
For example there’s a tool to calculate parameters here: http://sim.okawa-denshi.jp/en/Sallen3tool.php
For example you can select a Butterworth filter approximation and enter 15000 for the frequency and see what it produces.
And why are you using an LM324 in each stage of your output filter in the first place? You only need one op-amp, not four, and I suggested using an idealised op-amp model, as the purpose is just to suppress high-frequency components that would otherwise cause aliasing.
For example you could try OPAMP(TYPE=1 FPF=5 RI=1M RO=50 UGF=1M)
as a starting point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TDA2003 has a cutoff frequency of 15kHz – that might be a better choice.
This is (seemingly, unless I'm misreading the datasheet?) only true if the Cx and Rx components are populated, and in the case of Dribbling, this doesn't seem to be the case. I'm not sure if the TDA2003 circuit has any specific filter cutoff at all, without those components. Again, I could be wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that this works:
// 15KHz low-pass filter
OPAMP(AMP, "OPAMP(TYPE=1 FPF=5 RI=1M RO=50 UGF=1M SLEW=1M VLH=0.5 VLL=0.03 DAB=0.0015)")
I had to populate more parameters, but I used the ones from the example and it sounds well. But I am no expert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TDA2003 has a cutoff frequency of 15kHz – that might be a better choice.
This is (seemingly, unless I'm misreading the datasheet?) only true if the Cx and Rx components are populated, and in the case of Dribbling, this doesn't seem to be the case. I'm not sure if the TDA2003 circuit has any specific filter cutoff at all, without those components. Again, I could be wrong.
Nah, 15kHz is the typical -3dB frequency with the recommended R1/R2/R3/C2/C5, which are all included on the Dribbling schematic. You can add Rx and Cx to lower the upper cutoff frequency
The formula they give is Cx=1÷(2×π×B×R1) so B=1÷(2×π×Cx×R1). With the test circuit values of R1=220Ω and Cx=39nF, you get B=18.5kHz. It adds additional first-order roll-off above 18kHz, but it doesn’t change the -3dB point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TDA2003 has a cutoff frequency of 15kHz – that might be a better choice.
This is (seemingly, unless I'm misreading the datasheet?) only true if the Cx and Rx components are populated, and in the case of Dribbling, this doesn't seem to be the case. I'm not sure if the TDA2003 circuit has any specific filter cutoff at all, without those components. Again, I could be wrong.
The filter fixed the aliasing which is an emulator issue only though. Not really replacing the TDA2003 in strict sense.
Can you please slow down and take this one step at a time? It’s just going to take longer if you get ahead of yourself and keep making big changes like this. Let’s get the filtering sorted out first, then worry about the other issues. Please back out the comparator device. We’ll get to making a local approximation of the LM339 in your netlist later. Please read my other comment:
|
Done. I would say that, without a proper comparator, looking at JFET would be premature. How do we know they work properly if we use the LM324 instead? Put my attempt at a poor man comparator here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, this is looking better. I need a bit more time to get back to you on the next step. You’ll need to be patient, but I’m sure we’ll get to a state where it can be merged.
Okay, I am not impatient, it's just cognitive burden that I would love to get rid off ASAP. |
Rather than trying to write a model in C++, I’d suggest just building a simplified LM339 model in your netlist using the The
Things I’m not sure about:
Is that enough for you to use as a starting point? |
To be clear, I think that figure is too low. The output impedance should be higher than that since it acts as a current sink. |
Sounds good.
This is a great starting point and if there are no hidden traps (perf?) I can finish from it. In the meantime the full PARATA netlist is there for you to look for obvious mistakes or suggestions. |
It works. I compared the FUNC based comparator with the NATIVE one and the results are similar.
Native implementation:
Func implementation:
Input file:
I will replace the LM339 in the netlist with this one and generate the STOP_PALLA output next. |
Nope, when I plug it in in the STOP_PALLA netlist it keeps running forever. Replacing it with
I don't know why it's this slow. Given that I already implemented the native C++ can we switch to that and make it do the same thing this code does with LVCCS, etc.? |
Can you try this substitute LM339 model? It has a more detailed output stage. I tested it, and it seemed to sound OK. Hopefully it’s enough for now.
I did a quick test of the PARATA using NMOS depletion mode MOSFETs. Unfortunately getting it to work isn’t going to be as easy as I hoped. |
It doesn't sound OK, it sounds great. |
And we’re done. One more game with sound! |
I’m getting decent performance with a debug build using the static solvers on a Coffee Lake notebook. It should be OK for anyone with a reasonably up-to-date PC. |
If this runs great on an RPi5 especially with bgfx on, I am going to build a dedicated mini-cabinet for it. |
Continuation of PR 12127