Hi Tanu,
On Sun, Dec 18, 2016 at 08:59:58PM +0200, Tanu Kaskinen wrote:
> On Sat, 2016-12-17 at 01:43 +0200, Ahmed S. Darwish wrote:
> > # Note: Documentation of `pa_stream_begin_write(stream, &buf, &nbytes)'
> > # suggests to pass nbytes = -1. Doing so though lends to a huge returned
> > # nbytes values. Calling a pa_stream_write() with such values chokes the
> > # stream: making it go silent and not produce any further write-callback
> > # events.
> > #
> > # Is this a problem in usage pattern, or a bug?
>
> Sounds like a bug. Trying to write more than buffer_attr.maxlength
> bytes at once would be expected to cause some kind of problems, but I
> don't think that's the case here. The returned nbytes value is 64k,
> isn't it? pacat uses the default maxlength, and the default maxlength
> is 4M, so nbytes is nowhere near that limit. As long as maxlength isn't
> exceeded, any "extra" audio should just get buffered in the stream
> buffer at the server side.
>
Yeah; sounds like a bug in the write stream indeed.
In the command below, (second pacat):
./src/pacat -r --latency-msec=4 | ./src/pacat --latency-msec=4
When using "writable" values from pa_stream_writable_size(), we
get the following, valid, sequence:
++ STDIN callback: mute! stream writable = 0
@@ WRITE callback: Re-enable STDIN events
<-- Small writable val; appropriate for requested small latency -->
@@ STDIN callback: stream writable size = 192
** Writing 192 bytes; frame len = 4
++ STDIN callback: mute! writable = 0
@@ WRITE callback: Re-enable STDIN events
<-- Small writable val; appropriate for requested small latency -->
@@ STDIN callback: stream writable size = 192
** Writing 192 bytes; frame len = 4
... Sequence endlessly repeated ...
BUT when using "writable" values from pa_stream_begin_read(), with
"writable = (size_t)-1" as the documentation recommends, we get
that sequence instead:
@@ WRITE callback: Re-enable STDIN events
@@ STDIN callback: stream writable size = 65472 <== 64K indeed
** Writing 2112 bytes; frame len = 4
++ STDIN callback: mute! writable = 0
<-- COMPLETE silence! - no more stream write callback events -->
Hopefully not a new blocker, but if you agree with the assessment
above, I'll submit a bug to be tracked for v11.
...
> >
> > - buffer_length = (uint32_t) r;
> > - buffer_index = 0;
> > + /* Cache any trailing partial frames for the next write */
> > + partialframe_len = r - pa_frame_align(r, &sample_spec);
> > + if (partialframe_len)
> > + memcpy(partialframe_buf, buf + r - partialframe_len, partialframe_len);
> >
> > - if (w)
> > - do_stream_write(w);
> > + if (pa_stream_write(stream, buf, r - partialframe_len, NULL, 0, PA_SEEK_RELATIVE) < 0) {
> > + pa_log(_("pa_stream_write() failed: %s"), pa_strerror(pa_context_errno(context)));
> > + quit(1);
> > + return;
> > + }
>
> If we read less than a full frame, we can end up calling
> pa_stream_write() with zero length. That's quite possibly ok, but it
> makes me a bit nervous. I'd prefer cancelling the write if r -
> partialframe_len equals zero.
>
Agree. Updated in v3.
thanks!
--
Darwish
http://darwish.chasingpointers.com
