Dne 24. 03. 21 v 17:17 David Henningsson napsal(a):
>
> 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.
I see. I agree that the fixed frame is easier to handle.
> 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.
We can use the free bits in tv_nsec. It may be possible to carry 4 midi bytes
with the 64-bit tv_sec field, too.
Jaroslav
--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
