[PATCH v2] sound: rawmidi: Add framing mode

[Takashi Iwai]

On Wed, 24 Mar 2021 06:42:53 +0100,
David Henningsson wrote:
> 
> This commit adds a new framing mode that frames all MIDI data into
> 16-byte frames with a timestamp from the monotonic_raw clock.
> 
> The main benefit is that we can get accurate timestamps even if
> userspace wakeup and processing is not immediate.
> 
> Signed-off-by: David Henningsson <coding at diwic.se>

Thanks for the patch!
I seem to have overlooked your previous post, sorry.

This looks like a good middle ground solution, while we still need to
address the sequencer timestamp (basically we should be able to send
an event with the timestamp prepared from the rawmidi side).

The implementation itself looks good to me.  But this needs to bump
the SNDRV_RAWMIDI_VERSION for indicating the new API.


Takashi

> ---
> 
> v2: Fixed checkpatch errors.
> 
>  include/sound/rawmidi.h     |  1 +
>  include/uapi/sound/asound.h | 18 ++++++++++++++-
>  sound/core/rawmidi.c        | 45 ++++++++++++++++++++++++++++++++++++-
>  3 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/include/sound/rawmidi.h b/include/sound/rawmidi.h
> index 334842daa904..4ba5d2deec18 100644
> --- a/include/sound/rawmidi.h
> +++ b/include/sound/rawmidi.h
> @@ -81,6 +81,7 @@ struct snd_rawmidi_substream {
>  	bool opened;			/* open flag */
>  	bool append;			/* append flag (merge more streams) */
>  	bool active_sensing;		/* send active sensing when close */
> +	u8 framing;			/* whether to frame data (for input) */
>  	int use_count;			/* use counter (for output) */
>  	size_t bytes;
>  	struct snd_rawmidi *rmidi;
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index 535a7229e1d9..f33076755025 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -736,12 +736,28 @@ struct snd_rawmidi_info {
>  	unsigned char reserved[64];	/* reserved for future use */
>  };
>  
> +enum {
> +	SNDRV_RAWMIDI_FRAMING_NONE = 0,
> +	SNDRV_RAWMIDI_FRAMING_TSTAMP_MONOTONIC_RAW,
> +	SNDRV_RAWMIDI_FRAMING_LAST = SNDRV_RAWMIDI_FRAMING_TSTAMP_MONOTONIC_RAW,
> +};
> +
> +#define SND_RAWMIDI_FRAMING_DATA_LENGTH 7
> +
> +struct snd_rawmidi_framing_tstamp {
> +	unsigned int tv_sec;	/* seconds */
> +	unsigned int tv_nsec;	/* nanoseconds */
> +	unsigned char length;
> +	unsigned char data[SND_RAWMIDI_FRAMING_DATA_LENGTH];
> +};
> +
>  struct snd_rawmidi_params {
>  	int stream;
>  	size_t buffer_size;		/* queue size in bytes */
>  	size_t avail_min;		/* minimum avail bytes for wakeup */
>  	unsigned int no_active_sensing: 1; /* do not send active sensing byte in close() */
> -	unsigned char reserved[16];	/* reserved for future use */
> +	unsigned char framing;		/* For input data only, frame incoming data */
> +	unsigned char reserved[15];	/* reserved for future use */
>  };
>  
>  #ifndef __KERNEL__
> diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c
> index aca00af93afe..cd927ba178a6 100644
> --- a/sound/core/rawmidi.c
> +++ b/sound/core/rawmidi.c
> @@ -721,6 +721,7 @@ int snd_rawmidi_input_params(struct snd_rawmidi_substream *substream,
>  			     struct snd_rawmidi_params *params)
>  {
>  	snd_rawmidi_drain_input(substream);
> +	substream->framing = params->framing;
>  	return resize_runtime_buffer(substream->runtime, params, true);
>  }
>  EXPORT_SYMBOL(snd_rawmidi_input_params);
> @@ -963,6 +964,44 @@ static int snd_rawmidi_control_ioctl(struct snd_card *card,
>  	return -ENOIOCTLCMD;
>  }
>  
> +static int receive_with_tstamp_framing(struct snd_rawmidi_substream *substream,
> +			const unsigned char *buffer, int src_count, struct timespec64 *tstamp)
> +{
> +	struct snd_rawmidi_runtime *runtime = substream->runtime;
> +	struct snd_rawmidi_framing_tstamp frame;
> +	struct snd_rawmidi_framing_tstamp *dest_ptr;
> +	int dest_frames = 0;
> +	int frame_size = sizeof(struct snd_rawmidi_framing_tstamp);
> +
> +	frame.tv_sec = tstamp->tv_sec;
> +	frame.tv_nsec = tstamp->tv_nsec;
> +	if (snd_BUG_ON(runtime->hw_ptr & 15 || runtime->buffer_size & 15 || frame_size != 16))
> +		return -EINVAL;
> +
> +	while (src_count > 0) {
> +		if ((int)(runtime->buffer_size - runtime->avail) < frame_size) {
> +			runtime->xruns += src_count;
> +			return dest_frames * frame_size;
> +		}
> +		if (src_count >= SND_RAWMIDI_FRAMING_DATA_LENGTH)
> +			frame.length = SND_RAWMIDI_FRAMING_DATA_LENGTH;
> +		else {
> +			frame.length = src_count;
> +			memset(frame.data, 0, SND_RAWMIDI_FRAMING_DATA_LENGTH);
> +		}
> +		memcpy(frame.data, buffer, frame.length);
> +		buffer += frame.length;
> +		src_count -= frame.length;
> +		dest_ptr = (struct snd_rawmidi_framing_tstamp *) (runtime->buffer + runtime->hw_ptr);
> +		*dest_ptr = frame;
> +		runtime->avail += frame_size;
> +		runtime->hw_ptr += frame_size;
> +		runtime->hw_ptr %= runtime->buffer_size;
> +		dest_frames++;
> +	}
> +	return dest_frames * frame_size;
> +}
> +
>  /**
>   * snd_rawmidi_receive - receive the input data from the device
>   * @substream: the rawmidi substream
> @@ -977,6 +1016,7 @@ int snd_rawmidi_receive(struct snd_rawmidi_substream *substream,
>  			const unsigned char *buffer, int count)
>  {
>  	unsigned long flags;
> +	struct timespec64 ts64;
>  	int result = 0, count1;
>  	struct snd_rawmidi_runtime *runtime = substream->runtime;
>  
> @@ -988,7 +1028,10 @@ int snd_rawmidi_receive(struct snd_rawmidi_substream *substream,
>  		return -EINVAL;
>  	}
>  	spin_lock_irqsave(&runtime->lock, flags);
> -	if (count == 1) {	/* special case, faster code */
> +	if (substream->framing == SNDRV_RAWMIDI_FRAMING_TSTAMP_MONOTONIC_RAW) {
> +		ktime_get_raw_ts64(&ts64);
> +		result = receive_with_tstamp_framing(substream, buffer, count, &ts64);
> +	} else if (count == 1) {	/* special case, faster code */
>  		substream->bytes++;
>  		if (runtime->avail < runtime->buffer_size) {
>  			runtime->buffer[runtime->hw_ptr++] = buffer[0];
> -- 
> 2.25.1
>