Date 13.6.2014 10:44, Alexander E. Patrakov wrote:
13.06.2014 13:13, Jaroslav Kysela пишет:
Date 12.6.2014 12:34, Alexander E. Patrakov wrote:
This commit does not fix nonsense values returned by the rewind and forward callbacks. E.g., with period_size = 1024 and buffer_size = 4096, an attempt to rewind 1024 samples from the nearly-full buffer returns 4090.
Due to these nonsense values, the current rate plugin should be treated as non-rewindable. That's why the new callbacks return 0.
Signed-off-by: Alexander E. Patrakov patrakov@gmail.com
I don't agree. The snd_pcm_rate_move_applptr() should be fixed instead this blocking attempt. Do you have any simple use case?
I don't understand what you mean by a "simple use case".
I meant a test case (a simple test program).
And anyway, due to reasons expressed in http://permalink.gmane.org/gmane.linux.alsa.devel/122179 , I insist that 0 is the only correct return value even if someone fixes snd_pcm_rate_move_applptr(), which is not trivial. Also, 0 is not a regression - it was "crash" previously, and I have not changed the currently-broken snd_pcm_rate_move_applptr logic (just for the impossible case if someone does rely on it).
The full fix, after which I do agree to change 0 to something else, would involve:
- Writing a rewindable resampler library, or a resampler library that
offers a public API to save its state. 2. Dropping all other resampler implementations. 3. Extending the snd_pcm_rate_ops_t structure in order to forward either rewinds or save-requests to the resampler. 4. Using these new callbacks.
This translates to several months, or maybe a year of work at the current rate. It has to be done, and I do plan to do this. But it should not block the crash-fix: a suboptimal but valid return value of snd_pcm_rewindable() is better than a crash inside this function.
I see, I agree that the current code is/was tricky and probably broken as you noted.
But along with your patch, the code in rewind/forward callbacks should be also removed. I put yours and this patch to the alsa-lib repository:
http://git.alsa-project.org/?p=alsa-lib.git;a=commit;h=5256e150eb34cf599e798...
Thanks, Jaroslav