[RFC][icedtea6-pulse-audio]: Start recording corked.

[Denis Lila]

> Thanks for the explanation. Th patch itself looks good to me. Okay for
> HEAD.

Thanks for the review.
I pushed it.

I attached a new patch. This one should be the final* one.
It fixes the latency problem, which was the reason for this
whole thing.

ChangeLog:
2011-06-17  Denis Lila  <dlila at redhat.com>

    * pulseaudio/src/java/org/classpath/icedtea/pulseaudio/PulseAudioDataLine.java
    (bufferSize): Remove.
    (getBufferSize): Return stream.getBufferSize().
    * pulseaudio/src/java/org/classpath/icedtea/pulseaudio/PulseAudioSourceDataLine.java
    (connectLine): Improve formatting.
    * pulseaudio/src/java/org/classpath/icedtea/pulseaudio/PulseAudioTargetDataLine.java
    (connectLine): Set up flags to adjust the latency, if needed.
    * pulseaudio/src/java/org/classpath/icedtea/pulseaudio/Stream.java
    (bufAttr, bufAttrMutex): New members.
    (setBufAttr, bufferAttrCallback): New methods. They both set bufAttr.
    (getBufferSize): Return the current buffer size.
    (connectForRecording): Add a flags argument to allow callers to chose the 
    flags.
    (stateCallback): When the stream is ready, set the buffer attributes to
    the actual ones used by the server.
    * pulseaudio/src/native/org_classpath_icedtea_pulseaudio_Stream.c
    (buf_attr_changed_callback): New function. 
    (Java_org_classpath_icedtea_pulseaudio_Stream_native_1pa_1stream_1new):
    Set the buffer attribute callback.


* Barring any regressions. Or maybe we'll want to make further
improvements. For example, the bug was filed for recording, but
playback probably has the same problem and we might want to
make the fix work for both. Also, when reading data from
the native stream, there is more copying and allocation/gc
than there needs to be (the latter is probably not much of a
problem though): PATDL.read copies the pulse audio provided data
into a buffer, and then copies that into the user's buffer. This
can probably be avoided.

Regards,
Denis.

----- Original Message -----
> On 06/17/2011 03:41 PM, Denis Lila wrote:
> >> We now have two sets of constants for different purposes. Have you
> >> thought about prefixing them to indicate if they are stream flags
> >> or
> >> stream states?
> >
> > Good idea. Done. I had to add an argument to the macro that does
> > all the work though, to allow java names to have their own
> > prefixes.
> >
> >>>        const char* dev = NULL;
> >>>        if (device != NULL) {
> >>>            dev = (*env)->GetStringUTFChars(env, device, NULL);
> >>> @@ -468,10 +482,9 @@
> >>>                return -1; // oome thrown
> >>>            }
> >>>        }
> >>> - /* Set flags to 0 to fix problem with draining before calling
> >>> start, might need to
> >>> - be changed back to PA_STREAM_START_CORKED in the future, if
> >>> we'll
> >>> be able to implement
> >>> - synchronization*/
> >>
> >> Ah, so someone had set flags to 0 instead of PA_STREAM_START_CORKED
> >> explicitly. Did you test if this is still a problem?
> >
> > That comment was from connect_playback, not connect_record.
> > connect_playback did (and still does) start streams in
> > a corked state. The comment suggests that it used to start the
> > stream with NOFLAGS and that this should be changed once proper
> > synchronization was implemented. Since the flag was CORKED
> > what must have happened was that the author did implement
> > proper synchronization but just forgot to remove the comment
> > (and also forgot to start recording streams in the corked state).
> >
> > Anyway, I didn't test this case, because I didn't change any of
> > the playback logic, and because it's pretty clear that if drain()
> > is called before start() it will just return almost immediately.
> > The same is true of TDL.drain().
> >
> > I attached the updated patch.
> >
> 

> 
> Cheers,
> Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pa_latency.patch
Type: text/x-patch
Size: 8859 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110617/44a7fa13/pa_latency.patch