-
Notifications
You must be signed in to change notification settings - Fork 15
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
Additional optional sample rates in SoapySDRPlay driver #16
Comments
Hi Franco, Franco Spinelli logged a support ticket with this issue and you can get to lower arbitrary sample rates, but only in Zero-IF mode because you need a starting sample rate of between 2 and 10 MHz So 384 kHz = 3.072 MHz with a decimation of 8 and 768 kHz would be 3.072 MHz with a decimation of 4 etc. Obviously the only issue to be aware of with Zero-IF is the DC offset correction algorithm, so you would need an offset between the LO and the VFO for signals such as AM, although for FM this wouldn't be a problem. Hope that helps, Best regards, Andy |
@SDRplay The question that I'd like to discuss next is how to best implement this set (or range) of sample rates in the SoapySDRPlay driver, since I think most of the customers use it with the CubicSDR application, and I would like to avoid (or limit) changes to the way they currently see the RSP devices and their sample rates. There are two main places in the SoapySDRPlay driver where the client application deals with sample rates (just to make sure we are all on the same page, by 'sample rates' in this discussion I am always referring to the 'output sample rates', i.e. the sample rates as seen by the client, as per our discussion with Vincent a while ago):
In light of the current behavior, I was considering a few options (@frspin feel free to add your suggestions too):
Franco |
Hi Franco, I think there will always need to be a consideration between what we could do vs what the SoapySDR framework was designed to support. The classic case being the gain control issue. My understanding is that SoapySDR was really designed to support a range of predefined sample rates, hence why there is a listSampleRates function. This has presumably been modelled on the ExtIO format where the exact same thing is done. I would personally stick with the predefined list of sample rates which is easier to control and more in keeping with the SoapySDR framework. It is true to say that 2 MHz, 1 MHz, 500 kHz should all be delivered using Low-IF, beyond that the others you mentioned will all have to be Zero-IF. You really want to keep the sample rate as low as possible for 2 reasons...
As an algorithm, you can start with the final sample rate and keep multiplying by 2 until you get equal to or above 2 MHz and then that becomes your input sample rate. Does that stack up with what you want to do? Best regards, Andy |
Just my 2 cents here on CubicSDR behaviour:
So basically a user can input any sample rate he wants. Another small point, while I agree with @SDRplay for the others:
IMHO, that part is quite small compared to the other DSP processing done later by any SDR application, we should not limit the bandwith if it is beneficial. About the ADC bit part, I agree. Personnaly I stick to 8MHz all the time with my RSP2 because if gives the best apparent results: In particular, the other lower sample rates give worse images an dirtier signals, so I keep using 8MHz whatever the RF band I'm looking at. |
And therein lays the problem. What I see as a developer a lot of the time, is that each of us has an experience which is different to the other and sometimes that can "bleed" into the development. In truth the API can deal with any scenario you throw at it, the reason I prefer to use low sample rates is because I'm typically using SBCs where every bit of processing power is precious. Whilst Vincent uses 8 MHz sample rate because that suits his setup. Neither of those scenarios have really anything to do with the hardware or the API, it's either the host we choose to use or the environment (atmosphere, antenna, cable, interference, etc.) that mostly dominate. I don't really have a preference as to how the sample rate is delivered, if the SoapySDR/CubicSDR environment can cope for arbitary sample rates, then the API can deal with that if you choose to implement it. Just as a word of caution, when we started SDRuno, we gave the user the ability to tweak everything the API could do. That was a massive mistake, we have learned that people really want less options, but more optimisation for those options. What's really more important here than the sample rate, is what IF bandwidth you select. You have to avoid aliasing and make sure that the IF bandwidth is appropriate for the sample rate selected (whatever it ends up being). Best regards, Andy |
@SDRplay @vsonnier I think Andy's last comment about trying to keep things simple and give the user fewer but well tuned (and well tested) choices is very important. Also I suspect that most (if not all) of the users use one of the predefined settings for the sample rate, i.e. the 'manual' setting in CubicSDR mentioned by Vincent is seldom used (if at all). Please correct me if I am wrong; I'd like to hear what are the reasons for selecting an output sample rate of say 100kHz, or 495kHz. If these premises are correct (especially the one about 'manual' sample rates), I think the best approach to add support for 'linhpsdr' (and possibly other applications of the HPSDR family in the future) would be to just add the four sample rates: 96kHz, 192kHz, 384kHz, and 768kHz, as suggested by @frspin, and they would be implemented with a fsHz=3.072kHz, zero IF, and appropriate decimation factor. If we all agree on this approach, I can work on making these changes (in a new branch) over the weekend, we can run some tests with the various applications and use cases, and then make a final decision. Franco |
Sorry Franco, You're right, I should have said that 2 MHz, 1 MHz, 500 kHz, 250 kHz, 125 kHz, 62.5 kHz should all be done with Low-IF, the rest will need to be Zero-IF I've never used the arbitrary sample rate feature in CubicSDR - I didn't even know it existed or that the SoapySDR framework supported it! Best regards, Andy |
Well then, let me try to come at it from a different perspective. The focus on CubicSDR is not a good practise to start with, there's other applications out there where sample rates may have a completely different meaning: I'm talking about OpenWebRX, where the sample rate choice is mostly based on what band you're on and how much of the spectrum you'd like to share.
|
@jketterl As a developer it is very useful for me to see other people's perspectives, understand how the driver is used, and how to make it better and more useful. The way I see it is that the SoapySDRPlay driver (and the SoapySDR framework in general) is trying to abstract the internal complexity of the low level hardware, and provide a sort of unified 'experience' and API to the various applications that use it; in my mind, I see a stack more or less like this: a low level library that talks directly to the hardware (like libsdrplay_api or librtlsdr) at the bottom; an 'intermediate' level API (like SoapySDR, or a GnuRadio OOT module) that provides a well defined and unified API (and because of being unified, many times it is a simplified abstraction of the underlying low level API), and finally a client application on top, like OpenWebRX, or CubicSDR (or for instance gqrx in the case of GnuRadio). The important thing to notice here is that the intermediate level layer has sometime to make some trade-offs (or choices) between presenting a unified and consistent API to the client and the 'quirks' of the underlying library/hardware; case in point here is what to do with sample rates below or equal to 2MHz for the SDRplay RSPs: a request for say a sample rate of 500kHz could be implemented in two ways, with the LIF approach, or with the ZIF approach, but the setSampleRate() call does not have a way to specify that (and rightly so, because this is a high level API that shouldn't "bothered" with this kind of details), so the intermediate level driver has to manage this case in some way. This problem can also get even trickier when you try to implement a continuous range of sample rates, where say a request for a sample rate of 499.5kHz would be implemented one way (using ZIF), and a request for a sample rate of 500kHz would be implemented in a different way (using LIF) . A user playing around with the sample rates settings might change the sample rate from 500kHz to say 499.5kHz, notice that things are very different, and think your application has a bug because of that. Also we need to consider is that another useful purpose of this intermediate level is to simplify things to the client application (and therefore to the end user, since many client applications like CubicSDR present that list of sample rates directly to the user) in a way that the user does not have to "think too much" about what to select (like Andy pointed out in his comments about the evolution of the UX in SDRuno); presenting a well defined set of sample rate values all implemented in a well defined way could be more useful than returning a range of [min,max] and hoping for the best. Finally I agree 100% regarding printing an error message and returning an error for invalid sample rates; the problem in this case is that the Soapy driver API defines setSampleRate() as a function returning a void (see: https://github.com/pothosware/SoapySDR/blob/926c86d98b3a1d0b948ef6252982ee55230f1f0c/include/SoapySDR/Device.hpp#L873), so the developer is left to scratch their head on how to handle errors inside setSampleRate(): throw an exception? call abort()? (just kidding), perhaps there is a better way? This all said, I am not at all against adding other possible sample rates below 2MHz, if we know that they are useful and used in one of the many applications that use this driver; I think we can just come up with a list, explicitly state how to uniquely implement them (LIF for 62.5, 125, 250, etc; ZIF for the others), and I'll be more than happy to change the code to do that (of course the list of sample rates is not cast in stone; it could be modified in the future as we see fit). Franco |
The best approach, from my experience, is to just provide both, i.e. an API that provides reasonable defaults so you can get usable results in a very quick fashion, but then also expose the advanced settings for those that know what they're doing. Lower sample rates do make sense for OpenWebRX, especially on shortwave, where there is some very narrow HAM bands (the narrowest is 30m I believe, being only 50kHz wide). Some users have experienced aliasing issues with the lower sample rates, though (https://groups.io/g/openwebrx/topic/strange_behavior_with_rsp1/78060989). I haven't had much time to experiment with it, and maybe there is a way around this, but this suggests that there's a lower limit somewhere. The high sample rates, on the other hand, are useful for spanning multiple bands at the same time. The main use case for this is spotting / skimming applications, or maybe from 70cm upwards. Both require some CPU grunt, though, of course. As for the error handling: I did not know that the C++ API specifies a See: |
@jketterl On the high sample rates (i.e. above 2MHz) the current approach of fsHz=<sample rate>, Zero IF, and decimation factor=1 should work just fine, and should allow (at least in the call of the setSampleRate() function) to set any value you want. It's with the lower sample rates (<= 2MHz) that things get tricky. Another factor to consider, which I didn't mention in my previous comment, is that the RSPduo hardware in dual tuner or master/slave configuration has to work in LIF mode, i.e for that specific hardware in that specific configuration, the only sample rates possible are 62.5kHz, 125kHz, 250kHz, 500kHz, 1MHz, and 2MHz, and that's it. I plan to make the changes for the additional sample rates over the weekend, and put them in a different branch; after that, we can run some tests with different clients, different hardware, and different configurations, see how it goes, and see how this driver can be improved. Also if you know of other applications using this driver, I think it would be a good time to invite their developers to join this discussion, so that we can reach a large consensus on this subject. Franco |
Hello all, I've added a commit to CubicSDR : cjcliffe/CubicSDR@7b1956f This commit attempts to better manage the case when the user request a
Throwing execptions is bad because nothing in this API suggests that a call may fail in such brutal way. We cannot reasonnably force SoapySDR applications to wrap every API call into On the other hand, for each All the discrete sample rates of the RSP that cannot be applied exactly should indeed be implemented that way:
|
How can you expect to just call a method and not perform any kind of error handling? Since the API does not allow passing a result as a return value, exceptions are the only thing that I can think of on how to handle any kind of error. This is done in SoapySDR itself, too, see: here or here. I'm not aware of any guiding documentation for implementing device modules, if there's any it would certainly be helpful. So far I'm only reading the code, and there's multiple indications that exceptions will be and should be thrown. That's at the level of SoapySDR, so deviating here is a bad idea. As for your suggestion about guessing the sample rate: It would be a lot of work to implement it this way for me, and would still be confusing. As stated before, a corresponding error would work better for my application. |
This is not in the contract or in nowhere the general documentation. This is even worse than a return value, because we don't know the type of exceptions supposed to be thrown. And no,
Those examples are not that good. They cover device initialization and deinit, which may not fail except in rare circumstances. On the other hand, thowing exceptions all over the place while the device has actually been initialized properly, while the application has previously configured the driver according to its advised capabilities (valid sample rates in particular), is certainly not an expected runtime beahaviour, IMHO.
This is what the RSP driver does already choosing internal settings according to the requested What I'm proposing is to stick to predefined discrete sample rates < 2MHz by choosing the closest value with pre-known settings hard-coded to the driver. This is even the only possibility for Master-Slave configs when only some sample rates may be valid at all. At the end of the day @fventuri is the one doing the real work here, so choosing the right solution is up to him. My personnal conclusion is, as far as i can see of previous coments that handling an arbitrary sample rate < 2MHz is too difficult so for simplification sake should be rejected if not in pre-defined discrete values either :
|
I don't know the contract, nor where to find it. If somebody has it, please post it. If there's no contract, I suppose it's best to ask the guys at SoapySDR. After all, this is about implementing an API rather than defining it.
I agree. I was kind of confused when I saw the runtime exception there.
That's just code I have had a look at while working with SoapySDR. I can search for more, if you wish. Stating that device initialization may not fail however is very optimistic. |
Given all the ideas and suggestions, I thought it would be useful to proceed a step at a time, so for now I just implemented only those four additional sample rates (96kHz, 192kHz, 384kHz, 768kHz), and limited the possible sample values <=2Mhz to only the set of the previous Low IF values (62.5kHz, 125kHz, 250kHz, 500kHz, 1MHz, and 2MHz) plus the additional values. I decided for this approach (for now) for a couple of reasons:
As I said earlier, I am open to ideas and suggestions; I'd also like to hear Andy's @SDRplay opinion when he is back at the office on Monday. The branch with these changes is called 'extra_sample_rates' (https://github.com/SDRplay/SoapySDRPlay/tree/extra_sample_rates); I quickly tried it with the latest version of CubicSDR compiled from the current master, and it didn't crash, so that's a good sign :-) Also Franco @frspin found a bug with a case of streaming buffer overflow, where in same cases the buffer was getting resized to a value larger than its capacity, and therefore causing an invalid pointer and then a crash; this is now fixed, both in master and in the 'extra_sample_rates' branch. Give it a try when you have time, and let me know how it goes. Franco |
To be honest what I call 'contract' here is just the code coments in the API source code: https://github.com/pothosware/SoapySDR/wiki#client-api, I don't think there are other requirements. As you can see there is even a C-wrapper API where Too bad, because there is no standard SoapySDR exception so we a are stuck throwing I've done it for Cubic, but I still don't like it : I used a dirty |
Quick check with this group on the testing of these changes. Unless I hear any objections, I plan to merge these changes into master tomorrow evening. Also, if you are interested, we can open a new thread to discuss a proper way to handle errors in the SoapySDRPlay driver. Franco |
Thanks @fventuri, I didn't had time to test myself, but not changing the sample rate in case of unsupported value is probably the way to go. Speaking of error handling, look at that comment @jketterl and all, by PothosWare author himself: So, hem... I'll be damned apparently throwing exceptions is considered cool to report errors in a SoapySDR module. |
@vsonnier, since the topic of this issue (adding a few extra sample rates) and the right way of handling the errors are (almost) orthogonal, I suggest that first I complete this change by merging it tomorrow. After that I'd like to open a new issue about the error handling; there we can continue this discussion (if further discussion is needed), and I'll create a new branch with the code changes needed to implement throwing a Franco |
My previous remarks still stands though : I've not found (maybe yet) recomendations or would be-specs by @guruofquality on which exceptions are advised or not to throw, so the generic handling could only be by black-hole So if you decide to go the exception route, you'll have to care to make the Thanks for your great work ! |
I merged these new extra sample rates into master. I also created a new issue here: #19 to discuss how to best handle errors due to invalid selected values and/or bad return codes. I am going to leave this issue open for another few days in case there is a problem due to the additional sample rates; otherwise I would suggest that we continue the discussion about error handling in that thread. Franco |
@SDRplay
Andy, I am creating this new issue in the SoapySDRPlay driver to ask your opinion about the following question.
Franco Spinelli IW2DHW (@frspin) and I have been working for some time to get John Melton's G0ORX linphsdr program to work with the SDRplay RSPs using the SoapySDRPlay driver (see my pull request to Johns' repository here: g0orx/linhpsdr#84)
John's program uses a DSP library called WDSP (https://github.com/g0orx/wdsp) by Warren Pratt NR0V, that has been specifically designed for the HPSDR class of SDRs; these SDRs work with sample rates in the 48kHz 'family', i.e. 48kHz, 96kHz, 192kHz, and 384kHz.
Based on the discussions with you and Vincent a while ago (and based on what SDRuno presents to the user), I was under the impression that the RSPs would work best with output sample rates of 2MHz (and 1M, 500k, 250k, 125k, 62.5k using decimation) with an IF of 1620kHz, and with sample rates > 2MHz with a zero IF.
What we would be interested instead is to be able to use them with a sample rate of say 1536kHz and a zero IF (which with decimation would yield all the sample rates above, since 1536/4=384, 1536/8=192, and so on).
Franco (@frspin) modified his own copy of the SoapySDRPlay driver, ran same tests, and it looks like this configuration would work, but, not being familiar at all with the internals of the RSPs and the libsdrplay_api library, I wanted to ask your counsel before moving forward.
If you think these sample rate values are acceptable and do not cause other problems, I thought they could be added to the SoapySDRPlay driver, either in the main code, or perhaps using a 'cmake' compilation flag (like we used to to have for the RF gain thing a while ago), or perhaps in a different repo branch (which might make things a little more complicated since the two branches would have to be kept in sync after that). Another option would be for us to create a fork somewhere with these additional optional values of the sample rate, but I am afraid it could cause unnecessary confusion among the customers.
Thanks,
Franco
The text was updated successfully, but these errors were encountered: