Re: [PATCH v2 1/2] Add rawmidi framing API

[Takashi Iwai <tiwai@xxxxxxx>]

On Tue, 17 Aug 2021 14:51:31 +0200,
David Henningsson wrote:
> 
> --- a/include/rawmidi.h
> +++ b/include/rawmidi.h
> @@ -79,6 +79,27 @@ typedef enum _snd_rawmidi_type {
>  	SND_RAWMIDI_TYPE_VIRTUAL
>  } snd_rawmidi_type_t;
>  
> +#define SND_RAWMIDI_CLOCK_NONE		(0<<3)
> +#define SND_RAWMIDI_CLOCK_REALTIME	(1<<3)
> +#define SND_RAWMIDI_CLOCK_MONOTONIC	(2<<3)
> +#define SND_RAWMIDI_CLOCK_MONOTONIC_RAW	(3<<3)

IMO, better to be like below:

enum {
	SND_RAWMIDI_CLOCK_NONE = 0,
	SND_RAWMIDI_CLOCK_REALTIME = 1,
	SND_RAWMIDI_CLOCK_MONOTONIC = 2,
	SND_RAWMIDI_CLOCK_MONOTONIC_RAW = 3,
};

The shift should be done in the function.
The enum above can be also typedef'ed for the later reference.

> +typedef struct _snd_rawmidi_framing_tstamp {
> +	/**
> +	 * For now, frame_type is always 0. Midi 2.0 is expected to add new
> +	 * types here. Applications are expected to skip unknown frame types.
> +	 */
> +	__u8 frame_type;

The 0 frame type should be also defined explicitly, such as
SNDRV_RAWMIDI_FRAME_TYPE_XXX.

> +int snd_rawmidi_params_set_framing_type(const snd_rawmidi_t *rawmidi, snd_rawmidi_params_t *params, unsigned int val);

You can define the enum type to be more specific, too.

> +int snd_rawmidi_params_set_clock_type(const snd_rawmidi_t *rawmidi, snd_rawmidi_params_t *params, unsigned int val);

Ditto.


thanks,

Takashi