Re: [cp-patches] Patch: start of ALSA MIDI provider code
Hi,
On Sun, 2005-10-02 at 01:01 -0700, Anthony Green wrote:
Here's the start of ALSA MIDI provider code. MIDI IN ports basically
work. You can read and print events from a MIDI keyboard using the
standard interfaces. It's not perfect, but it's a start.
Cool stuff.
You call the jni library gjsmalsa, what does that stand for?
dnl ---
+dnl ALSA code (enabled by default)
+dnl ---
+AC_ARG_ENABLE([alsa],
+ [AS_HELP_STRING(--disable-alsa,compile ALSA providers
(disabled by --disable-alsa) [default=yes])],
+ [case ${enableval} in
+yes) COMPILE_ALSA=yes ;;
+no) COMPILE_ALSA=no ;;
+*) COMPILE_ALSA=yes ;;
+ esac],
+ [COMPILE_ALSA=yes])
+if test x$COMPILE_ALSA = xyes; then
+ AC_CHECK_HEADERS([alsa/asoundlib.h],,COMPILE_ALSA=no)
+fi
+AM_CONDITIONAL(CREATE_ALSA_LIBRARIES, test x${COMPILE_ALSA} = xyes)
I think it should say AS_HELP_STRING(--enable-alsa,... reading
--disable-alsa in the configure help output here is a but confusing.
Also when running configure with --disable-alsa I get:
configure: error: conditional AMDEP was never defined.
I couldn't figure out why.
Feel free to call the new native directory just 'alsa' or 'midialsa'
instead of the super long gnu-javax-sound-midi-alsa. That might get
boring and we don't have that much jni code that we really need to use
package namespacing to disambiguate things.
In your .c files please use the copyright boilerplate as is. So don't
reindent it. It should be similar to how you did the .java file
boilerplate.
Instead of things like /* Nothing yet. */ please use FIXME to help
people find unimplemented things. Although in general you should just
not stub things but leave them out for now.
Please don't use abort() or printf() but do something like:
JCL_ThrowException (env, java/lang/InternalError,
snd_strerror(error));
return;
The .java source files don't compile with gcj (4.0.2).
/usr/bin/gcj -Wno-deprecated --encoding=UTF-8 --bootclasspath '' --classpath
..:../vm/reference:..:../external/w3c_dom:../external/sax:.: -C -d . -MD -MF
lists/gnu-javax-sound.deps -MT lists/gnu-javax-sound.stamp -MP
@lists/gnu-javax-sound.list
../gnu/javax/sound/midi/alsa/AlsaPortDevice.java:91: error: Type 'Info' not
found in the declaration of the return type of method 'getDeviceInfo'.
public Info getDeviceInfo()
^
It does compile with jikes though. So this might be a gcj or build
infrastructure bug.
There is lots and lots of stuff that look autogenerated:
/* (non-Javadoc)
* @see javax.sound.midi.MidiDevice#getTransmitter()
*/
public Transmitter getTransmitter() throws MidiUnavailableException
{
// TODO Auto-generated method stub
return null;
}
Please don't include that (non-Javadoc) comment.
And are you sure you really need all this stuff already?
I really don't want to have so many stubs in our code. So if at all
possible just leave things out that are not necessary yet.
And please try to keep your lines 80 characters for readability.
Cheers,
Mark
signature.asc
Description: This is a digitally signed message part
___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] Patch: start of ALSA MIDI provider code
On Sun, 2005-10-02 at 19:45 +0200, Mark Wielaard wrote:
Cool stuff.
You call the jni library gjsmalsa, what does that stand for?
Gnu Javax Sound Midi ALSA. I also have a gjsmdssi for Gnu Javax
Sound Midi DSSI.
dnl ---
+dnl ALSA code (enabled by default)
+dnl ---
+AC_ARG_ENABLE([alsa],
+ [AS_HELP_STRING(--disable-alsa,compile ALSA providers
(disabled by --disable-alsa) [default=yes])],
+ [case ${enableval} in
+yes) COMPILE_ALSA=yes ;;
+no) COMPILE_ALSA=no ;;
+*) COMPILE_ALSA=yes ;;
+ esac],
+ [COMPILE_ALSA=yes])
+if test x$COMPILE_ALSA = xyes; then
+ AC_CHECK_HEADERS([alsa/asoundlib.h],,COMPILE_ALSA=no)
+fi
+AM_CONDITIONAL(CREATE_ALSA_LIBRARIES, test x${COMPILE_ALSA} = xyes)
I think it should say AS_HELP_STRING(--enable-alsa,... reading
--disable-alsa in the configure help output here is a but confusing.
Yes, I think so also. I was just copying what others had done.
Also when running configure with --disable-alsa I get:
configure: error: conditional AMDEP was never defined.
I couldn't figure out why.
I'll try to figure this out.
Feel free to call the new native directory just 'alsa' or 'midialsa'
instead of the super long gnu-javax-sound-midi-alsa. That might get
boring and we don't have that much jni code that we really need to use
package namespacing to disambiguate things.
Ok, I'll go with midi-alsa.
In your .c files please use the copyright boilerplate as is. So don't
reindent it. It should be similar to how you did the .java file
boilerplate.
Hmmm.. I copied that boilerplate from other .c files. Maybe I just
picked a bad one.
Instead of things like /* Nothing yet. */ please use FIXME to help
people find unimplemented things. Although in general you should just
not stub things but leave them out for now.
Please don't use abort() or printf() but do something like:
JCL_ThrowException (env, java/lang/InternalError,
snd_strerror(error));
return;
Right.
The .java source files don't compile with gcj (4.0.2).
/usr/bin/gcj -Wno-deprecated --encoding=UTF-8 --bootclasspath '' --classpath
..:../vm/reference:..:../external/w3c_dom:../external/sax:.: -C -d . -MD -MF
lists/gnu-javax-sound.deps -MT lists/gnu-javax-sound.stamp -MP
@lists/gnu-javax-sound.list
../gnu/javax/sound/midi/alsa/AlsaPortDevice.java:91: error: Type 'Info' not
found in the declaration of the return type of method 'getDeviceInfo'.
public Info getDeviceInfo()
^
It does compile with jikes though. So this might be a gcj or build
infrastructure bug.
This is probably a gcj bug. I'm building it with ecj. I'll tweak it to
build with gcj.
There is lots and lots of stuff that look autogenerated:
Yes, Eclipse is wonderful.
/* (non-Javadoc)
* @see javax.sound.midi.MidiDevice#getTransmitter()
*/
public Transmitter getTransmitter() throws MidiUnavailableException
{
// TODO Auto-generated method stub
return null;
}
Please don't include that (non-Javadoc) comment.
And are you sure you really need all this stuff already?
Actually this is correct code. I just need to remove the comments.
I really don't want to have so many stubs in our code. So if at all
possible just leave things out that are not necessary yet.
And please try to keep your lines 80 characters for readability.
I've tried my best. I'm not sure how to break up some of the deeply
nested code.
So, OK with these changes or do you want me to resubmit?
AG
___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] Patch: start of ALSA MIDI provider code
Hi Anthony, On Sun, 2005-10-02 at 11:47 -0700, Anthony Green wrote: So, OK with these changes or do you want me to resubmit? With these changes it is OK. But do post the final patch you want to commit for reference. Thanks, Mark signature.asc Description: This is a digitally signed message part ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] Patch: start of ALSA MIDI provider code
On Sun, 2005-10-02 at 17:10 -0700, Anthony Green wrote: This patch takes care of many of your comments, but not all. So the --disable and gcj compilation are fixed. It also adds a DSSI[1] provider. With this I was able to, in a few lines of Java code, connect my external MIDI keyboard to a DSSI soft-synth wired to jack[2] audio output and play music. There's still lots to do, but this is a nice milestone. And some cool new stuff was added :) Nice! If this patch isn't acceptable for the cvs trunk, would it be OK if I checked this into a new cvs branch? I feel uncomfortable keeping all this local. Now that the show stoppers are fixed you can check it in. That is useful because then others can play with it. But I do expect you to clean up the other things I mentioned. CVS shouldn't be something you dump half-finished stuff in without a clear plan how to get it finished. Thanks, Mark But I still don't like the non-Javadoc stuff it doesn't add anything useful. signature.asc Description: This is a digitally signed message part ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
