-
Notifications
You must be signed in to change notification settings - Fork 12
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
SimpleUSB: show RX level #411
Conversation
Note: minimal changes were needed for SimpleUSB as only one variable was involved. I think I can do the same for USBRadio but multiple settings (and calculations) are in play. Please advise if you think we parity between the two is essential. |
Not knowing the context, I think parity is a good thing in general, but I'll defer to @KB4MDD |
There are a number of adjustments in chan_usbradio. If you want to make the set Rx Voice Level the same as chan_simpleusb, that would be good. That would have those options the same and we could evaluate the need to add the current setting to the other commands. The P option can be used to see the current settings of the other items. |
I have updated the code so that both SimpleUSB and USBRadio will now display the current settings. |
I had noticed this inconsistency before, nice to see this now being added. Question though, is the rxvoiceadj parameter actually used in any audio processing? If so, where? I know the parameter exists and can be read/written to the conf file, but after what seemed a thorough search of the various C files it appears the variable is not used in any calculations. If that is correct, then it would not be very useful to show that cfg instead of rxmixerset in the rxdemod == RX_AUDIO_FLAT case. (From what I can tell, use of the rxvoiceadj parameter in USBRadio has no effect on any audio functionality and should be removed, and in SimpleUSB it has no significant effect other than causing clipping and should be removed from there also.) |
I see many references to o->rxvoiceadj in channels/chan_usbradio.c |
Hi Allen, as I mentioned above, "I know the parameter exists and can be read/written to the conf file, but after what seemed a thorough search of the various C files it appears the variable is not used in any calculations". My question is not if there are references to rxvoiceadj in the code. It is whether rxvoiceadj is actually used for anything that will have any effect whatsoever on audio levels or related USBRadio functionality. For example, if I add a parameter named xyz_adj to the config files, allow it to be read and written from the .conf files, and displayed and set in a utility menu, but that configuration parameter is not actually used in any way during processing of any audio, then what is the purpose of that parameter? Well of course it would have no purpose or reason to exist in the code if it doesn't actually have some real effect on some audio processing function or logic. From my review of the code, rxvoiceadj is not used in any way that would affect any audio or other processing done in USBRadio. If you disagree, could you please point out where this variable is used in such a way? Again, note that reading the parameter to/from a conf file, or EEPROM, or displaying it or editing it in a menu, does not mean that it actually affects actual channel driver audio/DSP processing functionality. Presumably rxvoiceadj was indeed used in some actual audio processing functionality at some point in the distant past, but it appears that whatever that functionality was, it has since been removed, but the rest of the code relating to the parameter was not. Thus it appears the parameter now does nothing but take up space. Therefore, rather than continue to copy and paste this parameter into other code, I would suggest we take a closer look at what it is actually doing, if anything. I'm pretty sure you will find that it is indeed not used for any real purpose, ie. it is a 'vestigial' parameter and any references to it in new or old code that imply it affects "Rx Voice Level" are incorrect. Maybe I'm wrong and my greps of the C files somehow missed it being assigned to some other variable or function that actually does use it in a way that affects Rx voice level, so in that case I apologize for asking you to show me where in the code this variable actually does something like that. However I'd bet $20 that this parameter does not in fact in any way affect rx voice levels, or anything else. I happened to have been looking at this parameter as part of my investigations into bug report #399, and noted there the following a couple days ago: "USBRadio does not appear to use the rxvoiceadj parameter to do any audio scaling. It references the parameter in a number of places, reading it and saving it to/from the conf files and optionally a CM1xx attached EEPROM, and printing the value in different places, but from what I can tell the value is never used in any kind of calculation or processing of audio. xpmr.c has a copy of rxvoiceadj (pChan->prxVoiceAdjust) but this also is never actually used. Thus it seems pretty clear to me that all code in chan_usbradio.c and xmpr.c referencing these variables is dead code, that just makes it harder for everyone to understand what the various channel driver settings and variables actually do, and that really should be deleted." |
Yes, looking more carefully and I agree. It looks as if the variable (and related usage) is only read or written but never actually used. Same for channels/xpmr/xpmr.c and channels/chan_voter.c. But, it does look like the variable is used in chan_simpleusb.c. But, having made the observation here, I don't think this is the place to drive its removal. Let's file another issue :-) |
Filed #417 |
You had added some comments to #399 that have me thinking. Specifically, that the CM108's variants allow setting the dBV values but not with the granularity of 0-999 levels that we have been presenting in the UI. To me, that means if I had been using a value of 500 but found the level to be just a bit low then I might raise it to 525 ... not knowing why raising the value by 10 can't be detected but raising the value by 15 could be heard/measured. Q? do you know if there's a place in the SimpleUSB / USBRadio channel drivers that knows what divisor is being used? I'm thinking about how we might display both the 0-999 value, the dBV value, and the interval between each step. With my latest, work-in-progress, changes I have the simpleusb-tune-menu showing :
Would this be better ?
But, how would one convey that you need to increment/decrement the level by 26.32 before it will make a difference? Would this be too confusing?
and, yes, we would be computing the low...high range for the current bucket Thoughts? |
Hi Allan, I think there are 2 steps that would be great to do, the first is to show the actual mixer settings read and written to the CM1xx, and then a 2nd more involved step would then be to fully characterize the A/AH vs. B CM1xx ICs and then also show what those mixer settings correspond to in actual voltages. The dBV values I mentioned before are what should be the case according to their datasheets (see https://allscan.info/images/CM1xx-specs-2000x2752.png), however there are some discrepancies in some parts of the datasheets as well as in actual observed behavior. I may have some time this week to run a script that loops through all the gain settings on both a CM108 AH and B while I apply test signals of known amplitudes to the Micin line, and confirm what signal levels actually result into ASL. To show the actual CM1xx mixer settings (eg. set in chan_simpleusb.c :: mixer_write()) I added the following to chan_simpleusb.c::susb_tune() after the first call to mixer_write(o);
With that in place when you then run the susbtune utility and change the rxmixerset value you'll see exactly what the setting gets translated to into the CM1xx. I'll do a bit more testing today on this and expand on this comment in a few hours but for now this should maybe help show the parameters of interest. |
I finished characterizing the CM108B. I calibrated a signal generator such that 0dB = the IC's max rated input which is 2.88Vpp. Then took the following measurements:
Conclusions: Will repeat this on the CM108AH tomorrow. Example of commands used:
The rx audio stats function comes in very handy for this kind of testing. It's also nice for showing distortion of a sine wave test signal, which should have exactly a 3dB difference in peak - Avg pwr. |
It seems that the reason C-Media went from 16 1.5dB ADC gain steps in A/AH to 36 1dB steps in the B's was to make it work more like a volume control rather than just a gain trim for boosting low level signals. (That also makes it more consistent with the DAC mixer range and steps.) For apps like ASL that don't have a 'volume' knob on the ADC side, the extra ~12dB of attenuation available in the B's does not translate to usable attenuation, though the input does seem to have some compression going on - with rxmixerset=0 I had to put 11.5Vrms into it (16.25Vpk), to get a max. reading of -7.2dBFS which is quite a lot of voltage to put into something that runs on a 5V supply. Each 1dB increase in rxmixerset then only yielded a ~0.6dB increase in the measured input peak value. Even with 16Vpk going in I had to set rxmixerset to 300 before I finally got a peak reading of 0dBFS. So there are some interesting nonlinearities happening with input levels > 6dBV. I'm going to try adding a more precise distortion measurement output for use with sine wave inputs which should clarify if there may be a valid use for the CM1xxB lower input gain settings. |
Looks like rxmixerset of 325 is the lowest setting that can be used where the full ADC dynamic range is available and no distortion occurs in the CM1xxB mixer. In the following test output I added a Crest Factor stat to the RxAudioStats output, and then for each gain setting I increase the sig gen input level until (pre-ADC) distortion or ADC clipping occurs. CF should be 3dB for a sine wave, and anytime goes below ~2.8dB but ClipCnt is 0 that indicates clipping/distortion in the CM1xxB mixer rather than in the ADC. I'll take a look at the IAX waveforms tomorrow also.
|
With the information you provided (and the suggested extra logging/reporting to the channel driver) it looks like it will be easy to know how the 0-999 levels are divided up. That means I can easily report :
What are your thoughts on "8/16" ? on "486...512"? Does either/both help? or do they make the level reporting too confusing? Reporting dBV may be out-of-reach, at least for this change ... but might be good to include in some documentation. |
Hi Allan, I just finished characterizing the CM108AH ADC. Results:
Conclusions: Example commands used during tests:
Summarizing the key points of all above testing,
I also plan to characterize the DAC side of the 3 main variants (CM108 A/AH, CM119A) as there is significant variation there in stated output range, and supposedly the same number of gain steps per the datasheets (0 - -45 dB), yet from above debug prints we see that AH has micplaymax 127 & spkrmax 151 vs. micplaymax 31 & spkrmax 37. The A/AH datasheets say nothing about having 151 txmix steps so I'll take a look at that. The DAC side is more straightforward. Options for what could be shown to users:
dBV units have all the advantages of dB but in addition correspond to actual voltages that can be easily measured with a DMM and cross-referenced to various radio or audio equipment datasheets. The CM1xx datasheets are unclear about this and have some contradictions. I'll plan to add these basic details to the ASL manual. dBV is the standard in pro audio equipment, most people won't know what exact voltage a certain dBV value corresponds to, but they wouldn't know that for an arbitrary 0-999 number either, and it's easy to look up a dBV value and see what it means, and dB units in general are more intuitive. Most hams are used to dB units such as with S-meters, and a good number of people have general familiarity with decibels and know for example that a -10dB difference makes something sound half as loud. dBV are simple to show, eg. |
The `simpleusb-tune-menu` shows the current TX levels but only reports the current RX level when adjustments are being made. The information is available and should be reported. Also, added parity between simpleusb and usbradio tune menus (the current settings will also be displayed by the latter).
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.
This looks good.
The
simpleusb-tune-menu
shows the current TX levels but only reports the current RX level when adjustments are being made. The information is available and should be reported.