[pulseaudio-discuss] [PATCH v2] pacat: Write to stream in frame-sized chunks

[Ahmed S. Darwish]

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