pulse-audio patches
Denis Lila
dlila at redhat.com
Wed Jun 15 14:21:15 PDT 2011
> > -
> > - 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
More information about the distro-pkg-dev
mailing list