[pulseaudio-discuss] Merging soxr

[Alexander E. Patrakov]

07.01.2015 02:36, Andrey Semashev wrote:
> On Tue, Jan 6, 2015 at 10:55 PM, Alexander E. Patrakov
> <patrakov at gmail.com> wrote:
>> 06.01.2015 19:17, Andrey Semashev wrote:
>>>
>>> As far as I understand the code, the loop is already there in
>>> pa_sink_input_peek() (see sink-input.c:924, "while (tchunk.length > 0)").
>>> The
>>> outer loop (sink-input.c:893, "while (!pa_memblockq_is_readable(i-
>>>>
>>>> thread_info.render_memblockq))") will end when either render_memblockq is
>>>> not
>>>
>>> empty or the sink input is drained; in the latter case render_memblockq
>>> can be
>>> empty.
>>
>> The "while (tchunk.length > 0)" loop calls the resampler until it eats all
>> of the available samples in tchunk. This looks insufficient, because the
>> testcase quoted above relies on the condition that the resampler eats all of
>> the available samples in tchunk (thus ending the loop), but produces
>> nothing.
>>
>>> However, render_memblockq is initialized with a silence chunk in
>>> pa_sink_input_new(), which means that when the queue is empty it should
>>> return
>>> silence. This means that pa_memblockq_peek() in sink-input.c:993 and the
>>> following asserts should never fail. Same for the asserts in
>>> fill_mix_info(),
>>> the loop should be cut short by pa_memblock_is_silence(). "pa_assert(info-
>>>>
>>>> chunk.memblock);" could be moved upper though, but it it doesn't matter
>>>> for
>>>
>>> the case in point.
>>
>> So, instead of an assertion failure that I predicted, we get a chunk of
>> silence regularly inserted into the middle of the low-latency stream being
>> resampled. Still bad. Thanks for correcting my logic, though.
>
> Yes, but it's not regularly as you say. In practice I've never heard
> anything like that (it would sound like audio cracking or something
> like that). I assume, soxr does produce audio for any practical amount
> of input samples (or there are other PA-specific factors in play). The
> silence would be generated while the resampler is filling, and during
> this period silence is exactly what is needed.

I will retest with qemu (via its alsa backend) later today.

> If no further modifications are needed to the surrounding code, are my
> patches sufficient or do I have to rebase or modify them?

Well, the patches still apply, so no rebasing is needed.

However, there are two cosmetical things and one disagreement about the 
documentation. So a resend with those issues fixed would be welcome.

1. Please squash PATCH 5/4 into PATCH 2/4.
2. In PATCH 1/4, in pa_resampler_soxr_init(), please sort the cases 
according to the upstream notion of quality, and make "default" the last 
case.
3. PATCH 4/4 reflects the upstream recommendation about quality, i.e. 
suggests that -hq is recommended and implies that there is some audible 
difference. However, my research suggests that all of the three 
remaining presets are perfect (and thus equal, indistinguishable) in 
terms of sound quality as perceived by humans when resampling from 44.1 
to 48 kHz, even on sound material specifically chosen to highlight the 
artifacts. Some wording is needed so that it is clear where the -hq 
recommendation comes from.

-- 
Alexander E. Patrakov