[alsa-devel] [PATCH] SND_SEQ_BLOCK and SND_SEQ_PORT_CAP_NONE defines
Hey hackers,
While developing pyalsa sequencer API, I found that sometimes 0 is used instead of a #defined value or constant.
In my opinion, given that the alsa API is opaque (hides internal structure and provides methods for accessing them), we should define those missing "constants" for being consistent. So far I found: SND_SEQ_BLOCK 0 (blocking mode) SND_SEQ_PORT_CAP_NONE (the port defines no access capabilites)
BTW, Thanks Jaroslav for putting my code; I will send 3rd patch for covering the missing items of the python API sequencer when I'll finish it.
Signed-off-by: Aldrin Martoq amartoq@dcc.uchile.cl --- diff --git a/include/seq.h b/include/seq.h --- a/include/seq.h +++ b/include/seq.h @@ -56,7 +56,8 @@ typedef struct _snd_seq snd_seq_t; /** * sequencer opening mode */ -#define SND_SEQ_NONBLOCK 0x0001 /**< non-blocking mode (flag to open mode) */ +#define SND_SEQ_BLOCK 0x0000 /**< blocking opening mode (flag for #snd_seq_open, #snd_seq_nonblock) */ +#define SND_SEQ_NONBLOCK 0x0001 /**< non-blocking opening mode (flag for #snd_seq_open, #snd_seq_nonblock) */
/** sequencer handle type */ typedef enum _snd_seq_type { @@ -204,6 +205,7 @@ typedef struct _snd_seq_port_info snd_se #define SND_SEQ_PORT_SYSTEM_ANNOUNCE 1 /**< system announce port */
/** port capabilities (32 bits) */ +#define SND_SEQ_PORT_CAP_NONE (0<<0) /**< port has no access capabilities */ #define SND_SEQ_PORT_CAP_READ (1<<0) /**< readable from this port */ #define SND_SEQ_PORT_CAP_WRITE (1<<1) /**< writable to this port */
diff --git a/src/seq/seq.c b/src/seq/seq.c --- a/src/seq/seq.c +++ b/src/seq/seq.c @@ -958,7 +958,7 @@ static int snd_seq_open_noupdate(snd_seq * \note Internally, these are translated to \c O_WRONLY, \c O_RDONLY and * \c O_RDWR respectively and used as the second argument to the C library * open() call. - * \param mode Optional modifier. Can be either 0, or + * \param mode Optional modifier. Can be either #SND_SEQ_BLOCK, or * #SND_SEQ_NONBLOCK, which will make read/write operations * non-blocking. This can also be set later using #snd_seq_nonblock(). * \return 0 on success otherwise a negative error code @@ -2161,8 +2161,9 @@ void snd_seq_port_info_set_timestamp_que * Each port has the capability bit-masks to specify the access capability * of the port from other clients. * The capability bit flags are defined as follows: + * - #SND_SEQ_PORT_CAP_NONE No access capabilities * - #SND_SEQ_PORT_CAP_READ Readable from this port - * - #SND_SEQ_PORT_CAP_WRITE Writable to this port. + * - #SND_SEQ_PORT_CAP_WRITE Writable to this port * - #SND_SEQ_PORT_CAP_SYNC_READ For synchronization (not implemented) * - #SND_SEQ_PORT_CAP_SYNC_WRITE For synchronization (not implemented) * - #SND_SEQ_PORT_CAP_DUPLEX Read/write duplex access is supported
At Thu, 31 Jan 2008 14:42:42 -0300, Aldrin Martoq wrote:
Hey hackers,
While developing pyalsa sequencer API, I found that sometimes 0 is used instead of a #defined value or constant.
In my opinion, given that the alsa API is opaque (hides internal structure and provides methods for accessing them), we should define those missing "constants" for being consistent. So far I found: SND_SEQ_BLOCK 0 (blocking mode) SND_SEQ_PORT_CAP_NONE (the port defines no access capabilites)
Well, I myself don't mind to add such defintions, but remember that these flags are no enum but bits. When you see BLOCK and NONBLOCK definitions, you'll likely think these are exclusive. However, (BLOCK|NONBLOCK) or (NONE|READ) can be passed without problems.
Takashi
BTW, Thanks Jaroslav for putting my code; I will send 3rd patch for covering the missing items of the python API sequencer when I'll finish it.
Signed-off-by: Aldrin Martoq amartoq@dcc.uchile.cl
diff --git a/include/seq.h b/include/seq.h --- a/include/seq.h +++ b/include/seq.h @@ -56,7 +56,8 @@ typedef struct _snd_seq snd_seq_t; /**
- sequencer opening mode
*/ -#define SND_SEQ_NONBLOCK 0x0001 /**< non-blocking mode (flag to open mode) */ +#define SND_SEQ_BLOCK 0x0000 /**< blocking opening mode (flag for #snd_seq_open, #snd_seq_nonblock) */ +#define SND_SEQ_NONBLOCK 0x0001 /**< non-blocking opening mode (flag for #snd_seq_open, #snd_seq_nonblock) */
/** sequencer handle type */ typedef enum _snd_seq_type { @@ -204,6 +205,7 @@ typedef struct _snd_seq_port_info snd_se #define SND_SEQ_PORT_SYSTEM_ANNOUNCE 1 /**< system announce port */
/** port capabilities (32 bits) */ +#define SND_SEQ_PORT_CAP_NONE (0<<0) /**< port has no access capabilities */ #define SND_SEQ_PORT_CAP_READ (1<<0) /**< readable from this port */ #define SND_SEQ_PORT_CAP_WRITE (1<<1) /**< writable to this port */
diff --git a/src/seq/seq.c b/src/seq/seq.c --- a/src/seq/seq.c +++ b/src/seq/seq.c @@ -958,7 +958,7 @@ static int snd_seq_open_noupdate(snd_seq
- \note Internally, these are translated to \c O_WRONLY, \c O_RDONLY and
- \c O_RDWR respectively and used as the second argument to the C library
- open() call.
- \param mode Optional modifier. Can be either 0, or
- \param mode Optional modifier. Can be either #SND_SEQ_BLOCK, or
- #SND_SEQ_NONBLOCK, which will make read/write operations
- non-blocking. This can also be set later using #snd_seq_nonblock().
- \return 0 on success otherwise a negative error code
@@ -2161,8 +2161,9 @@ void snd_seq_port_info_set_timestamp_que
- Each port has the capability bit-masks to specify the access capability
- of the port from other clients.
- The capability bit flags are defined as follows:
- #SND_SEQ_PORT_CAP_NONE No access capabilities
- #SND_SEQ_PORT_CAP_READ Readable from this port
- #SND_SEQ_PORT_CAP_WRITE Writable to this port.
- #SND_SEQ_PORT_CAP_WRITE Writable to this port
- #SND_SEQ_PORT_CAP_SYNC_READ For synchronization (not implemented)
- #SND_SEQ_PORT_CAP_SYNC_WRITE For synchronization (not implemented)
- #SND_SEQ_PORT_CAP_DUPLEX Read/write duplex access is supported
-- Aldrin Martoq _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Fri, 2008-02-01 at 13:38 +0100, Takashi Iwai wrote:
At Thu, 31 Jan 2008 14:42:42 -0300, Aldrin Martoq wrote:
Hey hackers, While developing pyalsa sequencer API, I found that sometimes 0 is used instead of a #defined value or constant. In my opinion, given that the alsa API is opaque (hides internal structure and provides methods for accessing them), we should define those missing "constants" for being consistent. So far I found: SND_SEQ_BLOCK 0 (blocking mode) SND_SEQ_PORT_CAP_NONE (the port defines no access capabilites)
Well, I myself don't mind to add such defintions, but remember that these flags are no enum but bits. When you see BLOCK and NONBLOCK definitions, you'll likely think these are exclusive. However, (BLOCK|NONBLOCK) or (NONE|READ) can be passed without problems.
I'm sorry, you are right and I was absolutely wrong. I thought mode is something like an enumeration rather than a flag (I think still there is a problem, for example there is no "change mode" api function, instead there is a snd_seq_nonblock function ?!); the port cap type was clearly a mistake.
I will correct this in the python version (remove this non-meaning constants) and please forget my patch. Thanks,
On Thu, 31 Jan 2008, Aldrin Martoq wrote:
Hey hackers,
While developing pyalsa sequencer API, I found that sometimes 0 is used instead of a #defined value or constant.
In my opinion, given that the alsa API is opaque (hides internal structure and provides methods for accessing them), we should define those missing "constants" for being consistent. So far I found: SND_SEQ_BLOCK 0 (blocking mode) SND_SEQ_PORT_CAP_NONE (the port defines no access capabilites)
Note that SND_SEQ_NONBLOCK is flag (bitmask). Also SND_SEQ_PORT_CAP_* defines are bitmasks. I don't see usage to have defined for 0 because it always mean "off state" rather than a direct value. Correct tests are:
val&SND_SEQ_NONBLOCK - nonblocking behaviour (val&SND_SEQ_NONBLOCK)==0 - blocking behaviour
Regarding python. I think that we need to settle naming scheme. I just moved my code to follow standard python rules (only class names have upper letters and constants).
I also chose a different organization for constants in my code to have possibility to enumerate all related bitmasks or numbers using dictionaries. See my python modules.
It meas to define SEQ_ADDRESS_* like:
seq_address = {'BROADCAST': 0xff, 'SUBSCRIBERS': 0xfe, 'UNKNOWN': 0xfd} open_mode = {'NONBLOCK': 1} etc...
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
On Wed, 2008-02-06 at 10:51 +0100, Jaroslav Kysela wrote:
On Thu, 31 Jan 2008, Aldrin Martoq wrote:
While developing pyalsa sequencer API, I found that sometimes 0 is used instead of a #defined value or constant. In my opinion, given that the alsa API is opaque (hides internal structure and provides methods for accessing them), we should define those missing "constants" for being consistent. So far I found: SND_SEQ_BLOCK 0 (blocking mode) SND_SEQ_PORT_CAP_NONE (the port defines no access capabilites)
Note that SND_SEQ_NONBLOCK is flag (bitmask). Also SND_SEQ_PORT_CAP_* defines are bitmasks. I don't see usage to have defined for 0 because it always mean "off state" rather than a direct value. Correct tests are: val&SND_SEQ_NONBLOCK - nonblocking behaviour (val&SND_SEQ_NONBLOCK)==0 - blocking behaviour
Yup, I was sorry for my mistake; I'm fixing (removing) this in my current python binding.
I think my mistake originated because there is no snd_seq_set_mode function API. The snd_seq_nonblock method is inconsistent with the flag behaivor. So, I'm sticking with changing the mode of a Sequencer instance through Sequencer.mode attribute.
Regarding python. I think that we need to settle naming scheme. I just moved my code to follow standard python rules (only class names have upper letters and constants).
Yeah, I think we should follow current python PEP's: a) Naming conventions http://www.python.org/dev/peps/pep-0008/ b) Doc strings http://www.python.org/dev/peps/pep-0257/
a) Says basically: - Class names are CapsWords; like in SeqEvent or SequencerError. - method names are lowercase and _ separated, like in get_mixer_controls(). - members of classes lowercase and _ separated. - There are no strict rules, so this are recommended practices. Sometimes, some modules follow other rules (ex: st_mode).
However, I couldn't find anything regarding constant naming in PEPs. I suggest: - keeping names all uppercase and _ separated. Must follow or imitate original C naming; like in SND_SEQ_NONBLOCK. - I'm not sure if keeping constants "inside" the module or make them exportable ("from alsaseq import *"). In the first way, we may follow python-gtk naming scheme (ex: gtk.gdk.AXIS_IGNORE); in the second way, we may stick to something like alsaseq.SND_SEQ_NONBLOCK or alsaseq.SEQ_NONBLOCK.
http://www.pygtk.org/docs/pygtk/gdk-constants.html
Jaroslav, What do you prefer?
I also chose a different organization for constants in my code to have possibility to enumerate all related bitmasks or numbers using dictionaries. See my python modules. It meas to define SEQ_ADDRESS_* like: seq_address = {'BROADCAST': 0xff, 'SUBSCRIBERS': 0xfe, 'UNKNOWN': 0xfd} open_mode = {'NONBLOCK': 1} etc...
I like your idea, and I'm thinking to change to that.
I first created the Constant class to force use them instead of an integer... The method was, raise a Warning error if the user didn't provide a Constant instance for some argument. That's why I implemented the NumProtocol for Constant: it returns a Constant for a bitwise between Constants... Then I realised this was too much strict type checking (like java) and not the python way, so I'm thinking of removing the numeric protocol for Constant. Note that the current code doesn't throw any warning.
However, I think a Constant class is useful, if you call __repr__ or __str__ to an instance it will return the name instead of looking through a dictionary.
Thanks for all your help, I'm still working for completing the full sequencer API. I'll be done/happy if a cover 80% of it: I've realized that open_lconf will need a new python binding for the Configuration API.
participants (3)
-
Aldrin Martoq -
Jaroslav Kysela -
Takashi Iwai