-
Notifications
You must be signed in to change notification settings - Fork 19
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
Documentation update following review #81
Conversation
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.
Just a couple of bits, fewer than last time.
Referenced to PDF from this run
- General
- "xcore.ai" incorrect throughout. Suggest find-replace XCORE-AI and xcore-ai to xcore.ai. Issue found on pages: 1, 2, 11, 19, 21
- Doxygen
ssrc_init
: "initialises" -> "initializes" (to match text and other function docs)asrc_init
: "initialises" -> "initializes"src_ds3_init
"initialises" -> "initializes"src_ds3_proc
"outputs one sample The input" Add full stop after "sample"src_os3_init
"initialises" -> "initializes"src_os3_input
"into the filter It should" Add full stop after "filter"src_os3_proc
"outputs one sample The input" Add full stop after "sample"
- Page 1
- "The library has no dependancies" -> dependencies
- Page 3
- "accessed via a standard function calls": remove "a"
- "in-line with your processing": 2nd person
- "within it's own task" -> its
- Page 4
- "are correct for your application": 2nd person
- Page 5
- "a single SRC instance, you can see": 2nd person
- Page 9
- Table 2.1: I still don't understand how to read this table at a glance. If the intention is that the top row of "44.1, 48, 88.2" etc. represent "Output rates" and the first column "44.1, 48, 88.2" etc. represents "Input rates" then I suggest a layout more along the lines of:
Output | ||||
44.1 | 48 | 88.2 | ||
Input | 44.1 | A | B | C |
48 | B | A | B | |
88.2 | C | B | A |
- ctd.
- "are a series FFT plots" -> "are a series of FFT plots"
- Page 11
- Tables 2.2 & 2.3: same comment as above
- "600 Mhz can provde" -> provide
- "is that SSRC uses" -> "the SSRC uses" OR "whereas the ASRC" -> "whereas ASRC".
- Page 12
- "The ASRC algorithm [...] The Bandwidth control" -> bandwidth
- Page 13
- "and applies them in the processing calls" -> apply
- Page 14
- "synchronous or over-sampling by 2 FIRs. It also provides" -> They also provide
- Page 16
- "The default setting is to use the 60dB [...]" Still clunky, suggest "to use the coefficients giving 60 dB [...]". Either way you feel about it, should be "60 dB", not "60dB".
- All instances of "us" must be "µs"
- "has been uninitialised and is NULL" -> uninitialized (to match rest of text)
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.
Nicely done. I still think the references to xcore.ai throughout are not correct, but bouncing it back for a 3rd revision feels gratuitious! Otherwise, looks great.
https://github.com/xmos/fwk_xvf/blob/a57a64ac37556305eac5afdf183d3101f59531b8/doc/user_guide/02_setting_up_the_hardware.rst?plain=1#L55