Re: [PATCH 1/1] sound: rawmidi: Add framing mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2021-03-24 17:06, Jaroslav Kysela wrote:
Dne 24. 03. 21 v 6:31 David Henningsson napsal(a):
This commit adds a new framing mode that frames all MIDI data into
16-byte frames with a timestamp from the monotonic_raw clock.
I would add support for monotonic timestamps, too. The NTP drifts are usually
small, so it may make sense to support those timestamps, too. It may be handy
for the synchronization among multiple machines (timing sources).

The timestamp mode should be selected separately than the framing mode.
Okay, noted for v3.

+#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];
+};
Perhaps, we should consider to have a fixed header and variable data length
here. For MIDI, the standard messages have only few bytes usually. It would be
better to use this space for the seconds field:

header {
	unsigned long long tv_sec;
	unsigned int tv_nsec;
	unsigned int len;
	unsigned char data[0];
};

I considered that, but it has problems with alignment. If you have a normal midi message of 3 bytes, now your second tv_sec will end up starting on an odd byte, unless you add padding, and then that padding needs to be specified and so on. In addition, half of the header could end up in the end of the ring buffer and the other half in the beginning. So I found the 16 byte fixed version to be simpler and easier to implement correctly.

However if you like we could change the tv_sec to 64 bit and end up with:

#define SND_RAWMIDI_FRAMING_DATA_LENGTH 3

struct snd_rawmidi_framing_tstamp {
	unsigned long long tv_sec;	/* seconds */
	unsigned int tv_nsec;	/* nanoseconds */
	unsigned char length;
	unsigned char data[SND_RAWMIDI_FRAMING_DATA_LENGTH];
};

We'll then have only three bytes for the actual data, but since that is what most midi messages are anyway, it would be okay, I assume.

// David





[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux