Skip to content
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

API Improvements: Document Requirements / Add Missing Constants #346

Closed
TheDaChicken opened this issue Jun 20, 2024 · 13 comments
Closed

API Improvements: Document Requirements / Add Missing Constants #346

TheDaChicken opened this issue Jun 20, 2024 · 13 comments

Comments

@TheDaChicken
Copy link
Contributor

TheDaChicken commented Jun 20, 2024

In the API,

nrsc5_pipe_samples_cs16 assumes input went through the half-band filter. This isn't documented as a requirement/function / is not labeled to require in the function name.

I encountered a problem using 16 bits as a constant type for piping data from different SDRs which required the half-band filter. I have to implement a half-band filter to pipe that type.

API does not include a few constants such as which may be required:

  • IQ sample rate
  • audio sample rate
  • audio sample count (provided in callbacks, not really needed)
  • audio channel count (not very important)

Silly small tidbit:
__declspec(dllexport) is meant to be used in the header file.

@argilo
Copy link
Collaborator

argilo commented Jun 20, 2024

nrsc5_pipe_samples_cs16 assumes input went through the half-band filter.

Actually there is no requirement to use a half-band filter. The only requirement is that the input sample rate is 744187.5. I agree that it would be useful to add the required sample rates to the API documentation (1488375 for nrsc5_pipe_samples_cu8, 744187.5 for nrsc5_pipe_samples_cs16).

We could also document that the audio sample rate is always 44100.

@argilo
Copy link
Collaborator

argilo commented Jun 20, 2024

The reason for 744187.5 is that it is the most convenient sample rate for OFDM demodulation. Unfortunately, the RTL-SDR does not natively support this sample rate (it allows only 225000-300000 and 900000-3200000). Because of this limitation, nrsc5 samples at 744187.5 * 2 = 1488375 and uses a decimating half-band filter to reach the desired sample rate. The API allows samples to be injected either before or after nrsc5's filter. If samples are injected after the filter, the application can use whatever means it likes to achieve the correct sample rate.

@TheDaChicken
Copy link
Contributor Author

Actually there is no requirement to use a half-band filter. The only requirement is that the input sample rate is 744187.5. I agree that it would be useful to add the required sample rates to the API documentation (1488375 for nrsc5_pipe_samples_cu8, 744187.5 for nrsc5_pipe_samples_cs16).

Thanks for letting me know. my oblivious nature is a good example that it should probably be documented

We could also document that the audio sample rate is always 44100.

Would be very useful. I have constants I added in my code that would make sense to be added.

@TheDaChicken
Copy link
Contributor Author

Should i make pull request for this?

@argilo
Copy link
Collaborator

argilo commented Jun 24, 2024

Yes, that would be useful.

@pclov3r
Copy link

pclov3r commented Jun 25, 2024

On the topic, It would be nice to see CS16 supported in NRSC5 for I/Q input without use the of API. #284

Also, Maybe some GNuradio flowgraphs for both cu8 and cs16 samples would be good to include?

I find myself taking I/Q files from say SDRconsole and running though GNUradio flowgraph to get a usable CU8 file for use in NRSC5. If I could keep those 16-bit I figure it should yield better performance.

@TheDaChicken
Copy link
Contributor Author

TheDaChicken commented Jun 25, 2024

I find myself taking I/Q files from say SDRconsole and running though GNUradio flowgraph to get a usable CU8 file for use in NRSC5. If I could keep those 16-bit I figure it should yield better performance.

A quick look, it looks like that would require another function in the API. For example something like: nrsc5_open_file_cs16
OR switch to loading the file in main.c? Would that be a good idea?

(directed towards argilo, wasn't clear when i made this post)

@pclov3r
Copy link

pclov3r commented Jun 25, 2024

I find myself taking I/Q files from say SDRconsole and running though GNUradio flowgraph to get a usable CU8 file for use in NRSC5. If I could keep those 16-bit I figure it should yield better performance.

A quick look, it looks like that would require another function in the API. For example something like: nrsc5_open_file_cs16 OR switch to loading the file in main.c? Would that be a good idea?

That would have to be @argilo call to make :)

Trying to create a gnuradio flowgraph now for cs16.

@pclov3r
Copy link

pclov3r commented Jun 25, 2024

I can't get CS16 input working with the the current Python cli in a file converted in GNUradio. Works perfectly fine for cu8 with flow graph over at #33 (comment)

I am feeding it 744187.5 resampled from GNUradio. Likely something I'm doing wrong.

Please excuse my poor DSP knowledge :)

Input sample rate here is 500,000 Singed 16-bit and the frequency doesn't need to be shifted.

image

@pclov3r
Copy link

pclov3r commented Jun 28, 2024

Your probably busy @argilo but when you get chance to answer the question form @TheDaChicken and my issue that would be awesome :)

@pclov3r
Copy link

pclov3r commented Jul 19, 2024

@argilo If your back from your vacation and can look at the above that would be awesome :)

@TheDaChicken Not sure if your done with #347 yet

@TheDaChicken
Copy link
Contributor Author

@TheDaChicken Not sure if your done with #347 yet

I'd say it is done. A new pull request can be done for #284

@TheDaChicken
Copy link
Contributor Author

The constants I wanted to document have been added. We can continue the IQ 16 bits discussion in #284

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants