Bug 47156

Created attachment 58405 [details] [review]
Drop the assert

Yup. Looking at the code, it would appear we can just drop the assert.

The return value in out_n_frames should still maintain proper integrity of the data.

I tend to agree that skipping the assert seems the easiest way out. 

There is a question though, what happens to the lost/unused sample? Will just skipping it lead to degrading quality? Ideally, it should be in the next block we send to libsamplerate, but fixing this seems tricky.

Also, it seems to happen very rarely, but maybe that's because we ship the speex resampler by default.

So either just skipping the assert, or possibly replace it with debug output telling us that x samples were skipped.

I'd very much like to avoid skipping any audio, and fixing this properly seems to be feasible. I'm working on a patch that stores the leftover data inside the resampler and adds the leftover to the input in the next pa_resampler_run() invocation.

Created attachment 60459 [details] [review]
[PATCH 1/3] resampler: Use pa_xnew0() to avoid manual zeroing.

Created attachment 60460 [details] [review]
[PATCH 2/3] resampler: Rename buf3 and buf4 to buf4 and buf5.

Created attachment 60461 [details] [review]
[PATCH 3/3] resampler: Add support for resamplers that consume less data than asked.

Updated patch set sent to the mailing list: http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/13091

The speex resampler may suffer from this same bug. There's a similar assertion for speex.

(In reply to comment #8)
> The speex resampler may suffer from this same bug. There's a similar assertion
> for speex.

Does the speex API actually imply that it may not consume all input data? The documentation suggests that this can only happen during an error:

"""
Unless an error occurs, either all input samples will be read or all output samples will be written to (or both).
"""

from: http://www.speex.org/docs/manual/speex-manual/node7.html#SECTION00760000000000000000

Patch set v3 sent to the mailing list: http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/13224

(In reply to comment #9)
> (In reply to comment #8)
> > The speex resampler may suffer from this same bug. There's a similar assertion
> > for speex.
> 
> Does the speex API actually imply that it may not consume all input data? The
> documentation suggests that this can only happen during an error:
> 
> """
> Unless an error occurs, either all input samples will be read or all output
> samples will be written to (or both).
> """
> 
> from:
> http://www.speex.org/docs/manual/speex-manual/node7.html#SECTION00760000000000000000

Thanks for checking the documentation. You are right. The speex changes need to be reverted.

Committed Tanu's patches for this.