On Mon, 19 Apr 2021 18:40:23 +0200, David Henningsson wrote:
This commit adds a new framing mode that frames all MIDI data into 32-byte frames with a timestamp.
The main benefit is that we can get accurate timestamps even if userspace wakeup and processing is not immediate.
Testing on a Celeron N3150 with this mode has a max jitter of 2.8 ms, compared to the in-kernel seq implementation which has a max jitter of 5 ms during idle and much worse when running scheduler stress tests in parallel.
Signed-off-by: David Henningsson coding@diwic.se
Changes since v5: Added realtime clock and changed params struct according to Jaroslav's wishes.
This version of the patch has been compile tested only.
Thanks for the updated patch. It looks almost good, just some minor issues below:
+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;
- u8 length; /* number of valid bytes in data field */
- u8 reserved[2];
- u32 tv_nsec; /* nanoseconds */
- u64 tv_sec; /* seconds */
- u8 data[SNDRV_RAWMIDI_FRAMING_DATA_LENGTH];
+} __attribute__((packed));
Use __packed instead.
@@ -720,7 +723,18 @@ EXPORT_SYMBOL(snd_rawmidi_output_params); int snd_rawmidi_input_params(struct snd_rawmidi_substream *substream, struct snd_rawmidi_params *params) {
- unsigned int framing = params->mode & SNDRV_RAWMIDI_MODE_FRAMING_MASK;
- unsigned int clock_type = params->mode & SNDRV_RAWMIDI_MODE_CLOCK_MASK;
- if (framing == SNDRV_RAWMIDI_MODE_FRAMING_NONE && clock_type != SNDRV_RAWMIDI_MODE_CLOCK_NONE)
return -EINVAL;- else if (clock_type > SNDRV_RAWMIDI_MODE_CLOCK_MONOTONIC_RAW)
return -EINVAL;- if (framing > SNDRV_RAWMIDI_MODE_FRAMING_TSTAMP)
return -EINVAL;- substream->framing = framing;
- substream->clock_type = clock_type; return resize_runtime_buffer(substream->runtime, params, true);
The stale framing and clock_type are set in the case of errors, which will confuse the later operations.
+static int receive_with_tstamp_framing(struct snd_rawmidi_substream *substream,
const unsigned char *buffer, int src_count, const struct timespec64 *tstamp)+{
....
- return dest_frames * frame_size;
The snd_rawmidi_receive() function is supposed to return the processed read size, hence this would become incompatible. Though, I don't find any code that actually checks the return value from snd_rawmidi_receive(), but a definition is a definition...
thanks,
Takashi