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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>


[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux