[RFC][icedtea6-pulse-audio]: Start recording corked.
Denis Lila
dlila at redhat.com
Fri Jun 17 12:41:24 PDT 2011
> 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.
Regards,
Denis.
----- Original Message -----
> On 06/16/2011 01:34 PM, Denis Lila wrote:
> > Hi.
> >
> > This patch makes only one functional change:
> > it starts recording streams corked (paused).
> > We used to start them uncorked on a TargetDataLine.open(...)
> > call, which was a waste because because a line
> > cannot be read until TargetDataLine.start() is called
> > on it. Therefore the start() corresponds to
> > uncorking the stream (which is reflected in
> > our implementation of start(), which just uncorks
> > the already uncorked stream).
> >
> > Other than this, the patch also moves pa_stream_flag_t
> > constants into java to give the java side control
> > over how streams are started. The implementation of
> > native_pa_stream_connect_playback no longer starts
> > the stream corked explicitly. Rather, it just passes
> > in the "flags" argument it received from its java
> > caller (which has set it to CORKED). These changes
> > are in line with the design of Stream.c, which has
> > very little logic, and it's mostly just a way for
> > the java code to make calls into pulse audio.
> >
> > ChangeLog:
> > 2011-06-16 Denis Lila<dlila at redhat.com>
> >
> > *
> > pulseaudio/src/java/org/classpath/icedtea/pulseaudio/Stream.java
> > (NOFLAGS, START_CORKED, INTERPOLATE_TIMING, NOT_MONOTONIC,
> > AUTO_TIMING_UPDATE, NO_REMAP_CHANNELS, NO_REMIX_CHANNELS,
> > FIX_FORMAT, FIX_RATE, FIX_CHANNELS, DONT_MOVE, VARIABLE_RATE,
> > PEAK_DETECT, START_MUTED, ADJUST_LATENCY, EARLY_REQUESTS,
> > DONT_INHIBIT_AUTO_SUSPEND, START_UNMUTED, FAIL_ON_SUSPEND):
> > New static long variables mirroring pa_stream_flag_t values.
> > (native_pa_stream_connect_playback,
> > native_pa_stream_connect_record):
> > Change flags parameter to long.
> > (connectForPlayback, connectForRecording): Start the stream
> > corked.
> > Change formatting to make it more readable.
> > *
> > pulseaudio/src/native/org_classpath_icedtea_pulseaudio_Stream.c
> > (SET_STREAM_ENUM): Renamed from SET_STREAM_STATE_ENUM, since
> > the
> > macro could have been used for any PA_STREAM constants, not
> > just
> > stream states (and indeed, we now use it for flag constants
> > too).
> > (Java_org_classpath_icedtea_pulseaudio_Stream_init_1constants):
> > Initialize flag constants in addition to the stream states.
> > (Java_org_classpath_icedtea_pulseaudio_Stream_native_1pa_1stream_1connect_1playback):
> > Change flags parameter to jlong (from jint), remove commented
> > out
> > dead code, remove obsolete comment, and start the stream with
> > whatever
> > flags were passed in the flags parameter, instead of ignoring
> > that
> > parameter and using PA_STREAM_START_CORKED.
> > (Java_org_classpath_icedtea_pulseaudio_Stream_native_1pa_1stream_1connect_1record):
> > Change flags parameter to jlong (from jint), remove commented
> > out
> > dead code.
> >
> >
>
> Comments inline, below.
>
> >
> > diff -r 2eef438960e4
> > pulseaudio/src/java/org/classpath/icedtea/pulseaudio/Stream.java
> > ---
> > a/pulseaudio/src/java/org/classpath/icedtea/pulseaudio/Stream.java
> > Thu Jun 16 11:11:35 2011 -0400
> > +++
> > b/pulseaudio/src/java/org/classpath/icedtea/pulseaudio/Stream.java
> > Thu Jun 16 12:56:32 2011 -0400
> > @@ -101,15 +101,12 @@
> >
> > // see comments in ContextEvent.java and Operation.java
> > // These are the possible stream states.
> > - // TODO: perhaps we should do this for stream flags too.
> > public static long UNCONNECTED = -1,
> > CREATING = -1,
> > READY = -1,
> > FAILED = -1,
> > TERMINATED = -1;
> >
> > - private static native void init_constants();
> > -
> > // Throw an IllegalStateException if value is not one of the
> > possible
> > // states. Otherwise return the input.
> > public static long checkNativeStreamState(long value) {
> > @@ -120,6 +117,29 @@
> > return value;
> > }
> >
> > + // These are stream flags.
> > + public static long NOFLAGS = -1,
> > + START_CORKED = -1,
> > + INTERPOLATE_TIMING = -1,
> > + NOT_MONOTONIC = -1,
> > + AUTO_TIMING_UPDATE = -1,
> > + NO_REMAP_CHANNELS = -1,
> > + NO_REMIX_CHANNELS = -1,
> > + FIX_FORMAT = -1,
> > + FIX_RATE = -1,
> > + FIX_CHANNELS = -1,
> > + DONT_MOVE = -1,
> > + VARIABLE_RATE = -1,
> > + PEAK_DETECT = -1,
> > + START_MUTED = -1,
> > + ADJUST_LATENCY = -1,
> > + EARLY_REQUESTS = -1,
> > + DONT_INHIBIT_AUTO_SUSPEND = -1,
> > + START_UNMUTED = -1,
> > + FAIL_ON_SUSPEND = -1;
> > +
>
> > 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?
>
> > diff -r 2eef438960e4
> > pulseaudio/src/native/org_classpath_icedtea_pulseaudio_Stream.c
> > ---
> > a/pulseaudio/src/native/org_classpath_icedtea_pulseaudio_Stream.c
> > Thu Jun 16 11:11:35 2011 -0400
> > +++
> > b/pulseaudio/src/native/org_classpath_icedtea_pulseaudio_Stream.c
> > Thu Jun 16 12:56:32 2011 -0400
> > @@ -453,14 +475,6 @@
> > buffer_attr.prebuf = (uint32_t) bufferPreBuffering;
> > buffer_attr.minreq = (uint32_t) bufferMinimumRequest;
> >
> > - /*
> > - printf("buffer maxlength: %u\n", buffer_attr.maxlength);
> > - printf("buffer tlength: %u\n", buffer_attr.tlength);
> > - printf("buffer prebuf: %u\n", buffer_attr.prebuf);
> > - printf("buffer minreq: %u\n", buffer_attr.minreq);
> > - printf("buffer fragsize: %u\n", buffer_attr.fragsize);
> > - */
> > -
>
> Any reason you are removing this?
>
>
> > - int value = pa_stream_connect_playback(stream, dev,&buffer_attr,
> > PA_STREAM_START_CORKED, NULL, sync_stream);
> > +
> > + int value = pa_stream_connect_playback(stream, dev,&buffer_attr,
> > + (pa_stream_flags_t) flags, NULL, sync_stream);
> >
>
> Everything else looks fine.
>
> Cheers,
> Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pa_start_recording_corked.patch
Type: text/x-patch
Size: 18085 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110617/f6894c03/pa_start_recording_corked.patch
More information about the distro-pkg-dev
mailing list