2012/5/2 Tanu Kaskinen <[email protected]>: > 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 <[email protected]>: >> > > 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 > _______________________________________________ pulseaudio-discuss mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
