[pulseaudio-discuss] [PATCH v2 3/3] resampler: Add support for resamplers that consume less data than asked.

[Wang Xingchao]

2012/5/2 Tanu Kaskinen <tanuk at iki.fi>:
> On Wed, 2012-05-02 at 06:08 +0300, Tanu Kaskinen wrote:
>> On Wed, 2012-05-02 at 10:44 +0800, Wang Xingchao wrote:
>> > Hi Tanu,
>> >
>> > 2012/4/25 Tanu Kaskinen <tanuk at iki.fi>:
>> > > libsamplerate_resample() assumed that src_process() would
>> > > always consume the whole input buffer. That was an invalid
>> > > assumption leading to crashes.
>> > >
>> > > This patch adds a leftover memchunk for storing any
>> > > non-consumed input. When pa_resampler_run() is called next
>> > > time, the leftover is prepended to the new input.
>> > >
>> > > Changes in v2:
>> > >  - If add_leftover() is called with zero-length input while
>> > >   the leftover length is non-zero, we don't try to acquire
>> > >   the input memblock.
>> > >  - Instead of taking a reference to the original input in
>> > >   libsamplerate_resample(), we copy the leftover data to a
>> > >   new memblock. This is done, because otherwise, if the
>> > >   input is one of the internal buffers, the data can get
>> > >   overwritten before reading it in add_leftover().
>> > >  - Store add_leftover_buf size in bytes instead of samples
>> > >   (more convenient, but less consistent with other code).
>> > >
>> > > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=47156
>> > [snip]
>> >
>> > > +static pa_memchunk *add_leftover(pa_resampler *r, pa_memchunk *input) {
>> > > +    void *src;
>> > > +    void *dst;
>> > > +
>> > > +    pa_assert(r);
>> > > +    pa_assert(input);
>> > > +
>> > > +    /* Concatenate leftover and input and place the result in
>> > > +     * add_leftover_buf. */
>> > > +
>> > > +    if (r->leftover_buf.length <= 0)
>> > > +        return input;
>> > > +
>> > > +    r->add_leftover_buf.length = r->leftover_buf.length + input->length;
>> > > +
>> > > +    if (!r->add_leftover_buf.memblock || r->add_leftover_buf_bytes < r->add_leftover_buf.length) {
>> > > +        if (r->add_leftover_buf.memblock)
>> > > +            pa_memblock_unref(r->add_leftover_buf.memblock);
>> > > +
>> > > +        r->add_leftover_buf_bytes = r->add_leftover_buf.length;
>> > > +        r->add_leftover_buf.memblock = pa_memblock_new(r->mempool, r->add_leftover_buf.length);
>> > > +    }
>> > > +
>> > > +    src = (uint8_t *) pa_memblock_acquire(r->leftover_buf.memblock);
>> > > +    dst = pa_memblock_acquire(r->add_leftover_buf.memblock);
>> > > +    memcpy(dst, src, r->leftover_buf.length);
>> > > +    pa_memblock_release(r->leftover_buf.memblock);
>> > > +
>> > > +    if (input->length > 0) {
>> > > +        src = (uint8_t *) pa_memblock_acquire(input->memblock) + input->index;
>> > > +        dst = (uint8_t *) dst + r->leftover_buf.length;
>> > > +        memcpy(dst, src, input->length);
>> > > +        pa_memblock_release(input->memblock);
>> > > +    }
>> > > +
>> >
>> > it's a bit different with original
>> > patch(https://bugs.freedesktop.org/attachment.cgi?id=60461). :)  i
>> > guess most of time the leftover buffer length should be little, and
>> > the main input buffer is bigger, so there're two times waste copy  and
>> > why not reduce that to one time copy?
>>
>> I don't understand what you mean. How would you achieve only one copy?
>>
>> If you mean that the leftover buffer should be copied to the input
>> buffer, thus avoiding copying the input buffer, that doesn't work. The
>> input buffer doesn't have any spare room for the leftover data.
>
> I now realized that copying the leftover data twice (once when storing
> it and once in add_leftover()) could be reduced to only one copy by
> using the same buffer in those two phases. The leftover size tends to be
> small, so the amount of work that is saved is also small, but I think it
> wouldn't make the code significantly more complex, so maybe I should
> implement that.

Sorry for confuse first.
Not exactly. If there's left_over bytes, we could put them at the
start of r->buf2, and change the "dst" of do_remap for next memchunk.
Is that okay for you?

--xingchao

>
> --
> Tanu
>