xmos / lib_tsn

Time sensitive networking library
Other
48 stars 29 forks source link

Documentation Incomplete: audio_output_fifo_pull_sample(...) #27

Closed ahogen closed 7 years ago

ahogen commented 7 years ago
  1. This documentation block (shown below) is inconstant with the actual function.
  2. Why are all functions/tasks defined in .c or .xc except for this one which is in .h? Please be consistent.
  3. There is a static inline unsigned int declaration of this function in the corresponding .c file, with no implementation defined. I've removed it in my own code without any issues. Why is it there?
/**
 *  \brief Used by the audio output system to pull the next sample from the FIFO
 *
 *  If there are no samples in the buffer, a zero will be returned. The current
 *  ref clock time is passed into the function, and the FIFO will record this
 *  time if the sample which has been removed was the marked sample
 *
 *  \param s the FIFO to remove a sample from
 *  \param timestamp the ref clock time of the sample playout
 */
 /*
unsigned int
audio_output_fifo_pull_sample(buffer_handle_t s,
                                  unsigned int timestamp);
*/
__attribute__((always_inline))
unsafe static inline unsigned int
audio_output_fifo_pull_sample(buffer_handle_t s0,
                              unsigned index,
                              unsigned int timestamp)

Applies to:

larry-xmos commented 7 years ago

Thanks for reporting the outdated comment, I will repair it

audio_output_fifo_pull_sample is in a header file for performance reasons. With the way XC compiler currently handles inlining, we found we had to use the always_inline attribute to get it to always inline into audio_buffer_manager as needed.

I suspect the empty inline definition is there just to follow a convention. You would typically write that to give yourself a non-inlined version of the function to link against later should the compiler decide that the inline version isn't used at compile time and discard it. In this instance the empty definition can be safely removed, as you found.

ahogen commented 7 years ago

Resolved by @larry-xmos in commit https://github.com/xmos/lib_tsn/commit/f4d5c83ffbe05114927495edae0a65b1061b2481