pulse-audio patches
Denis Lila
dlila at redhat.com
Thu Jun 16 07:42:24 PDT 2011
I attached the patch.
----- Original Message -----
> > > -
> > > - public ContextEvent(Type type) {
> > > - this.type = type;
> > > + // Throws an IllegalStateException if value is not one of the
> > > above
> > > + // context events. We do this for all pulse audio enumerations
> > > that
> > > + // we need to use in the java side. If pulse audio decides to
> > > add
> > > + // new events/states, we need to add them to the classes. The
> > > exception
> > > + // will let us know.
> > > + // return the input if there is no error
> >
> > Perhaps you should make this a javadoc?
>
> Done.
>
> > > diff -r 7e19bc7875bb -r 25d50d852479
> > > pulseaudio/src/native/jni-common.h
> > > --- a/pulseaudio/src/native/jni-common.h Mon Jun 13 12:02:38 2011
> > > -0400
> > > +++ b/pulseaudio/src/native/jni-common.h Tue Jun 14 15:53:31 2011
> > > -0400
> > > @@ -39,11 +39,20 @@
> > > #define _JNI_COMMON_H
> > >
> > > #include<jni.h>
> > > +#include<stdio.h>
> >
> > Is this needed?
> >
> > > diff -r 7e19bc7875bb -r 25d50d852479
> > > pulseaudio/src/native/org_classpath_icedtea_pulseaudio_Stream.c
> > > ---
> > > a/pulseaudio/src/native/org_classpath_icedtea_pulseaudio_Stream.c
> > > Mon Jun 13 12:02:38 2011 -0400
> > > +++
> > > b/pulseaudio/src/native/org_classpath_icedtea_pulseaudio_Stream.c
> > > Tue Jun 14 15:53:31 2011 -0400
> > > @@ -131,6 +131,7 @@
> > > assert(context->env);
> > > assert(context->obj);
> > >
> > > + PA_SAMPLE_ALAW;
> >
> > Stray line?
>
> Yeah, these were both debugging leftovers. I've removed them.
>
> > I think mirroring the other names, as in SET_STREAM_STATE_ENUM,
> > might
> > make the intent of this macro more clear.
>
> Done.
>
> ChangeLog:
> 2011-06-15 Denis Lila <dlila at redhat.com>
>
> * Makefile.am: Add ContextEvent to the list of pulse audio classes
> that
> need javah run on them.
> *
> pulseaudio/src/java/org/classpath/icedtea/pulseaudio/ContextEvent.java
> (Type): Remove and replace with...
> (UNCONNECTED, CONNECTING, AUTHORIZING, SETTING_NAME, READY, FAILED,
> TERMINATED): New static long variables replacing enum Type.
> (init_constants): New native method to initialize the above variables.
> (checkNativeEnumReturn): Make sure that the input is one of the longs
> representing the type of ContextEvent.
> (type): Change type from Type to long.
> (ContextEvent): Take a long argument, instead of a Type.
> (getType): Return a long, not a Type.
> * pulseaudio/src/java/org/classpath/icedtea/pulseaudio/EventLoop.java:
> (status): Change from int to long.
> (native_set_sink_volume): Remove. It was unimplemented in the JNI
> side.
> (getStatus): Return long instead of int.
> (update): Replace int argument with long argument. Remove the switch
> statement.
> (setVolume): Remove. Unused.
> * pulseaudio/src/java/org/classpath/icedtea/pulseaudio/Operation.java:
> (State): Remove and replace with...
> (Running, Done, Cancelled): Static longs, enumerating the possible
> operation states.
> (init_constants): New native method to initialize the above variables.
> (checkNativeOperationState): Make sure that the input is one of the
> longs
> representing the operation state.
> (native_get_state): Change return type from int to long.
> (getState): Change return type to long; remove switch.
> *
> pulseaudio/src/java/org/classpath/icedtea/pulseaudio/PulseAudioDataLine.java:
> Remove the names of enums from the names of constants since most of
> them
> were changed to static longs.
> *
> pulseaudio/src/java/org/classpath/icedtea/pulseaudio/PulseAudioMixer.java:
> Same changes as in PulseAudioDataLine.java.
> * pulseaudio/src/java/org/classpath/icedtea/pulseaudio/Stream.java:
> (State): Remove and replace with...
> (UNCONNECTED, CREATING, READY, FAILED, TERMINATED): New static long
> variables
> replacing enum Type.
> (init_constants): New native method to initialize the above variables.
> (checkNativeStreamState): Make sure that the input is one of the longs
> representing the kind of StreamState.
> (native_pa_stream_get_state): Change the return from int to long.
> (getState): Remove the switch.
> * pulseaudio/src/native/jni-common.h:
> (SET_JAVA_STATIC_LONG_FIELD_TO_PA_ENUM): Macro that sets one of the
> java
> static longs to the corresponding pa_constant.
> *
> pulseaudio/src/native/org_classpath_icedtea_pulseaudio_ContextEvent.c:
> New file.
> (SET_CONTEXT_ENUM): Macro that sets the ContextEvent types.
> (Java_org_classpath_icedtea_pulseaudio_ContextEvent_init_1constants):
> Implementation of ContextEvent.init_constants.
> * pulseaudio/src/native/org_classpath_icedtea_pulseaudio_EventLoop.c:
> (context_change_callback): Change the fourth argument of GetMethodID
> to "(J)V" to reflect the change in the signature of EventLoop.update.
> * pulseaudio/src/native/org_classpath_icedtea_pulseaudio_Operation.c:
> (SET_OP_ENUM): Macro that sets the operation types.
> (Java_org_classpath_icedtea_pulseaudio_Operation_init_1constants):
> Implementation of Operation.init_constants.
> (Java_org_classpath_icedtea_pulseaudio_Operation_native_1get_1state):
> Change return type to jlong.
> * pulseaudio/src/native/org_classpath_icedtea_pulseaudio_Stream.c:
> (SET_STREAM_ENUM): Macro that sets the stream states.
> (Java_org_classpath_icedtea_pulseaudio_Stream_init_1constants):
> Implementation of Stream.init_constants.
> (Java_org_classpath_icedtea_pulseaudio_Stream_native_1pa_1stream_1get_1state):
> Change return type to jlong.
>
>
>
> Thank you,
> Denis.
>
> ----- Original Message -----
> > On 06/14/2011 05:37 PM, Denis Lila wrote:
> > > Hi.
> > >
> > > I would like to (eventually) push the attached patches.
> > > These are not ready to go in yet, because they require
> > > the changeset in this e-mail
> > > http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2011-June/014722.html
> > >
> > > Therefore, this is just a preview I'm sending out
> > > because I don't want to work too long without any
> > > reviews.
> > >
> >
> > I have a few comments in-line below. But overall, the changes look
> > fine.
> >
> > > rev2597.patch replaces a few enums with static longs.
> > > This was done to make the java side more robust to
> > > changes in the native code or pulse audio. The longs
> > > allow us to initialize them (using the jni) to the
> > > values pulse audio uses for its own enums, so we no
> > > longer have to have switch statements that translate
> > > between java enum instances and ints.
> > >
> > > rev2598.patch removes a needless notifyAll()
> > > and fixes a couple of race conditions between
> > > read() and flush() in PulseAudioTargetDataLine.
> > >
> > >
> >
> >
> > >
> > > # HG changeset patch
> > > # User Denis Lila<dlila at redhat.com>
> > > # Date 1308086602 14400
> > > # Node ID b006f1d14da5e0254a2382e8f628a4d2b872fb90
> > > # Parent 25d50d852479eb2311da0deb3005fbfe9fb2ee15
> > > Concurrency problems and improvements.
> > >
> >
> > ... snip ...
> >
> > >
> > > icedtea6_rev2597.patch
> > >
> >
> > This patch looks fine to me.
> >
> > >
> > > # HG changeset patch
> > > # User Denis Lila<dlila at redhat.com>
> > > # Date 1308081211 14400
> > > # Node ID 25d50d852479eb2311da0deb3005fbfe9fb2ee15
> > > # Parent 7e19bc7875bb8265f3fd1ca69cc89a40eaad664b
> > > Use longs instead of enums for pulse audio enums.
> > >
> > > diff -r 7e19bc7875bb -r 25d50d852479
> > > pulseaudio/src/java/org/classpath/icedtea/pulseaudio/ContextEvent.java
> > > ---
> > > a/pulseaudio/src/java/org/classpath/icedtea/pulseaudio/ContextEvent.java
> > > Mon Jun 13 12:02:38 2011 -0400
> > > +++
> > > b/pulseaudio/src/java/org/classpath/icedtea/pulseaudio/ContextEvent.java
> > > Tue Jun 14 15:53:31 2011 -0400
> > > @@ -37,32 +37,64 @@
> > >
> > > package org.classpath.icedtea.pulseaudio;
> > >
> > > +import java.util.Arrays;
> > > +
> > > /**
> > > * This class encapsulates a change in the PulseAudio's
> > > connection
> > > context.
> > > *
> > > * When this event is fired, something has happened to the
> > > connection of this
> > > * program to the PulseAudio server.
> > > - *
> > > */
> > > -
> > > +// TODO: maybe make this class an enum with a field that
> > > indicates
> > > the kind of
> > > +// event?
> > > class ContextEvent {
> > > -
> > > + // There are certain enumerations from pulse audio that we need
> > > to
> > > use in
> > > + // the java side. For all of these, we declare static longs in
> > > the
> > > proper
> > > + // java classes and we use static native methods to initialize
> > > them to
> > > + // the values used by pulse audio. This makes us immune to
> > > changes
> > > in
> > > + // the integer values of the enum symbols in pulse audio.
> > > /**
> > > * Basically, what is the new state of the context
> > > - *
> > > + * These will be initialized to the proper values in the JNI.
> > > */
> > > - public static enum Type {
> > > - UNCONNECTED, CONNECTING, AUTHORIZING, SETTING_NAME, READY,
> > > FAILED,
> > > TERMINATED
> > > + static long UNCONNECTED = -1,
> > > + CONNECTING = -1,
> > > + AUTHORIZING = -1,
> > > + SETTING_NAME = -1,
> > > + READY = -1,
> > > + FAILED = -1,
> > > + TERMINATED = -1;
> > > +
> > > + private static native void init_constants();
> > > +
> > > + static {
> > > + SecurityWrapper.loadNativeLibrary();
> > > + init_constants();
> > > }
> > >
> > > - private Type type;
>
> >
> > > diff -r 7e19bc7875bb -r 25d50d852479
> > > pulseaudio/src/java/org/classpath/icedtea/pulseaudio/PulseAudioMixer.java
> > > ---
> > > a/pulseaudio/src/java/org/classpath/icedtea/pulseaudio/PulseAudioMixer.java
> > > Mon Jun 13 12:02:38 2011 -0400
> > > +++
> > > b/pulseaudio/src/java/org/classpath/icedtea/pulseaudio/PulseAudioMixer.java
> > > Tue Jun 14 15:53:31 2011 -0400
> > > @@ -701,7 +701,7 @@
> > > try {
> > > // System.out.println("waiting...");
> > > ready.acquire();
> > > - if (eventLoop.getStatus() != 4) {
> > > + if (eventLoop.getStatus() != ContextEvent.READY) {
> >
> > Wow, I cant believe I wrote this line. Thanks for fixing it!
> >
>
> >
> > > if (pa_stream_get_state(stream) == PA_STREAM_CREATING) {
> > > callJavaVoidMethod(context->env, context->obj,
> > > "overflowCallback");
> > > } else {
> > > @@ -240,6 +241,23 @@
> > >
> > > }
> > >
> > > +#define SET_STREAM_STATE(env, clz, state_name) \
> > > + SET_JAVA_STATIC_LONG_FIELD_TO_PA_ENUM(env, clz, STREAM,
> > > state_name)
> > > +
> >
>
> >
> > Everything else looks fine.
> >
> > Cheers,
> > Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pa_first.patch
Type: text/x-patch
Size: 25531 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110616/08625f9a/pa_first.patch
More information about the distro-pkg-dev
mailing list