On 2015-11-02 08:50, Andrey Semashev wrote:
> On Monday, November 02, 2015 08:33:47 AM David Henningsson wrote:
>> On 2015-10-31 16:59, Andrey Semashev wrote:
>>> The 0.1.2 version of libsoxr fixes soxr_process() crash after soxr_clear()
>>> is used, so check the library version at compile time and use
>>> soxr_clear() if possible.
>> Thanks for the patch, but if there is a bug in libsoxr that is fixed in
>> 0.1.2 and above, wouldn't make more sense just to depend on libsoxr >=
>> 0.1.2 in the configure script?
>>
>> Is there a good reason for supporting older libsoxr versions?
>
> Well, the code worked fine with 0.1.1 previously. There really is no major
> changes that require 0.1.2, I'm just removing the workaround that was
> previously needed.
>
> Also that's the version that is shipped in the latest Ubuntu, for instance. I
> consider 0.1.1 wide spread, so it's good to allow people to compile the latest
> PA on the current systems.
>
> If you don't want to keep support for 0.1.1 I can change the patch.
>
>>> ---
>>>
>>> src/pulsecore/resampler/soxr.c | 15 ++++++++-------
>>> 1 file changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/pulsecore/resampler/soxr.c
>>> b/src/pulsecore/resampler/soxr.c index b5f0007..b1b2e19 100644
>>> --- a/src/pulsecore/resampler/soxr.c
>>> +++ b/src/pulsecore/resampler/soxr.c
>>> @@ -64,17 +64,17 @@ static void resampler_soxr_free(pa_resampler *r) {
>>>
>>> }
>>>
>>> static void resampler_soxr_reset(pa_resampler *r) {
>>>
>>> +#if SOXR_THIS_VERSION >= SOXR_VERSION(0, 1, 2)
>>> + pa_assert(r);
>>> +
>>> + soxr_clear(r->impl.data);
>>> +#else
>>> + /* With libsoxr prior to 0.1.2 soxr_clear() makes soxr_process()
>>> crash afterwards, + * so don't use this function and re-create the
>>> context instead. */
>>
>> Also, you don't seem to have added code to re-create the context?
>
> The code for re-creating the context is already there. That's how it worked
> unconditionally before the patch.
Ah, sorry for not looking more thoroughly into it. I think your patch is
good as it is, so I pushed it now (after adding "soxr:" to the commit
message).
In a few years - or next time we need to refactor it - it might make
sense to skip the < 0.1.2 support for simplicity.
Thanks for the contribution!
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
