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

[Tanu Kaskinen]

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.

-- 
Tanu