Dne 29.12.2017 (pet) ob 15:40 +0200 je Tanu Kaskinen napisal(a):
> On Sat, 2017-12-16 at 16:57 +0100, Samo PogaÄ?nik wrote:
> >
> > +static ssize_t pipe_sink_write(struct userdata *u, pa_memchunk
> > *pchunk) {
> > +Â Â Â Â size_t index, length;
> > +Â Â Â Â ssize_t count = 0;
> > +Â Â Â Â void *p;
> > +
> > +Â Â Â Â pa_assert(u);
> > +Â Â Â Â pa_assert(pchunk);
> > +
> > +Â Â Â Â index = pchunk->index;
> > +Â Â Â Â length = pchunk->length;
> > +Â Â Â Â p = pa_memblock_acquire(pchunk->memblock);
> > +
> > +Â Â Â Â for (;;) {
> > +Â Â Â Â Â Â Â Â ssize_t l;
> > +
> > +Â Â Â Â Â Â Â Â l = pa_write(u->fd, (uint8_t*) p + index, length, &u-
> > >write_type);
> > +
> > +Â Â Â Â Â Â Â Â pa_assert(l != 0);
> > +
> > +Â Â Â Â Â Â Â Â if (l < 0) {
> > +Â Â Â Â Â Â Â Â Â Â Â Â if (errno == EAGAIN)
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> > +Â Â Â Â Â Â Â Â Â Â Â Â else if (errno != EINTR) {
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (!u->fifo_error) {
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â pa_log("Failed to write data to FIFO: %s",
> > pa_cstrerror(errno));
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â u->fifo_error = true;
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â }
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â count = l - count;
> process_render_use_timing() assumes that l is -1, so you should
> replace
> l with -1 here.
>
Agreed.
> >
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> > +Â Â Â Â Â Â Â Â Â Â Â Â }
> > +Â Â Â Â Â Â Â Â } else {
> > +Â Â Â Â Â Â Â Â Â Â Â Â count += l;
> > +Â Â Â Â Â Â Â Â Â Â Â Â index += l;
> > +Â Â Â Â Â Â Â Â Â Â Â Â length -= l;
> > +
> > +Â Â Â Â Â Â Â Â Â Â Â Â if (length <= 0) {
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> > +Â Â Â Â Â Â Â Â Â Â Â Â }
> > +Â Â Â Â Â Â Â Â }
> > +Â Â Â Â }
> > +
> > +Â Â Â Â pa_memblock_release(pchunk->memblock);
> > +
> > +Â Â Â Â if (u->fifo_error && count >= 0) {
> > +Â Â Â Â Â Â Â Â pa_log("Recovered from FIFO error");
> > +Â Â Â Â Â Â Â Â u->fifo_error = false;
> > +Â Â Â Â }
> This logic is not right. If in the first loop iteration pa_write()
> does
> a partial write and then an error occurs during the second iteration,
> the code incorrectly thinks that we have recovered from the error.
> The
> recovery happens when pa_write() returns a positive number for the
> first time.
>
This case is correctly covered, since count always get negative value
in the case of a fifo error, so immediate recovery can not occure via
this condition.
> Also, use pa_log_debug() or pa_log_info(). pa_log() is an alias for
> pa_log_error(), and this is not an error message.
>
Ok.
> >
> > +Â Â Â Â return count;
> > +}
> > +
> > +static int process_render_use_timing(struct userdata *u, pa_usec_t
> > now) {
> The function return type can be changed to void.
>
Can switch return type back to void - i just left the code prepared for
potential error case, that would require full pipe_sink failure -
module unload.
regards and thanks for the reply,
Samo
