xmos / lib_i2s

I2S/TDM digital audio interface library
Other
20 stars 26 forks source link

Examples need updating to explicitely set return for restart_check() to I2S_NO_RESTART if not already set. #116

Closed QuinnWang closed 2 years ago

QuinnWang commented 2 years ago

By using this one: https://github.com/xmos/sw_usb_audio/blob/develop/app_usb_aud_xk_evk_xu316_extrai2s/src/extensions/extra_i2s.xc#L87

If there is no restart = I2S_NO_RESTART at the server side, the client side restart_check() will be blocked for a while. Take the sample rate 48kHz as an example, without restart = I2S_NO_RESTART at server side, added 2 timer reading in the client side after reading the lrclk value, the error timing pattern will be: ... 6250 3746 1044 ...

tried to comment out the restart = i2s_i.restart_check(); in the i2s frame slave code, the time period pattern becoming as expected: ... 1044 1040 1043 1040 1043 1040 ...

so looks the blocker is at the restart_check(). After added restart = I2S_NO_RESTART in the i2s_data(), case i_i2s.restart_check() -> i2s_restart_t restart, the problem solved...

Here just want to consult you with the reason. And if the restart = I2S_NO_RESTART must be added, then suggested to add in all the example code to avoid encountering the error by other people, such as the lib_i2s-develop/examples/app_simple_i2s_frame_slave/src/simple_i2s_frame_slave.xc

Thanks in advance.

ACascarino commented 2 years ago

Hi Quinn, While this is unexpected behaviour, it's possible that this is a bug in most examples - if an interface callback doesn't explicitely set its return value it's a possibility that the result could be undefined - it might just be that on previous testing we've got lucky and it's returned 0. I'm unsure on this but for safety we should update all the examples to explicitely set the callback return value so we can avoid this issue in the future - thanks for spotting it! A