Re: [PATCH v2] sound: rawmidi: Add framing mode

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

 



Hi Takashi and Takashi,

On 2021-03-26 08:55, Takashi Iwai wrote:
On Fri, 26 Mar 2021 05:46:15 +0100,
Takashi Sakamoto wrote:
Hi David,

On Wed, Mar 24, 2021 at 04:57:31PM +0100, David Henningsson wrote:
However, the timing jitter of IRQ handler invocation is issued in this
case, as well as PCM interface, even if the data rate of MIDI physical
layer is quite low nowadays (31.25 Kbit / sec ~= 3906.25 byte / sec).
As long as I experienced, in actual running Linux system, the invocation
of IRQ handler has no guarantee for timing jitter, mainly due to CPU level
IRQ mask (like spin_lock). Therefore the interval of each invocation is not
so precise to decide event timestamp, at least for time slot comes from
MIDI physical layer.

Nevertheless, I think your idea is enough interesting, with conditions to
deliver information from driver (or driver developer) to applications
(ALSA Sequencer or userspace applications). Even if we have some
limitation and restriction to precise timestamp, it's worth to work for
it. It seems to be required that improvements at interface level and
documentation about how to use the frame timestamp you implemented.
Right, so first, I believe the most common way to transport midi these days
is through USB, where the 31.25 Kbit/sec limit does not apply: instead we
have a granularity of 1/8 ms but many messages can fit in each one. So that
limit is - for many if not most cases - gone.

Second; you're probably right in that there is still some jitter w r t when
the IRQ fires. That is regrettable, but the earlier we get that timestamp
the better, so a timestamp in IRQ context would at least be better than in a
workqueue that fires after the IRQ, or in userspace that possibly has
scheduling delays.

Btw, I checked the "struct urb" and there was no timestamp in there, so I
don't know how to get a better timestamp than in snd_rawmidi_receive. But
maybe other interfaces (PCI, Firewire etc) offers something better.
Hm. Regardless of type of hardware; e.g. OHCI/EHCI/xHCI, 1394 OHCI, or
PCI-e extension card, for software to process sampled data, it's always
issue that the jitter between triggering IRQ (hardware side) and invoking
IRQ handler (processor side). As an actual example, let me share my
experience in 1394 OHCI.


1394 OHCI controller is governed by 24.576 Mhz clock (or double depending
on vendors). In IEEE 1394, ishcornous service is 8,000 times per second.
1394 OHCI specification allows software to schedule hardware IRQ per
isochronous cycle.

Once ALSA firewire stack is programmed to schedule the hardwar IRQ evenry
16 isochronous cycle. The corresponding IRQ handler is expected to invoke
every 2 milli second. As long as I tested in usual desktop environment[2],
the jitter is between 150 micro second and 4.7 milli second. In the worst
case, it's 6.0 milli seconds. The above is also the same wnen using
'threadirqs'.

When using 'isolcpus' and arranging 'smp_affinity' to reduce load from one
of processor core to invoke the IRQ handler, the interval is 2 milli
second with +- several nano seconds, therefore the 1394 OHCI controller
can trigger hardware IRQ so precise. The jitter comes from processor side.
I think many running contexts on the same processor core masks IRQ so
often and the jitter is not deterministic.

Here, what I'd like to tell you is that we can not handle the system time
as is as timestamp of received MIDI messages, as long as relying on IRQ
context. We need some arrangements to construct better timestamp with some
compensations. In this point, the 3rd version of patch is not enough[3],
in my opinion.

Interesting measurements. While several ms of jitter is not ideal, I have a few counter arguments why I still believe we should merge this patch:

 1) I don't think we should let the perfect be the enemy of the good here, just because we cannot eliminate *all* jitter does not mean we should not try to eliminate as much jitter as we can.

 2) an unprivileged process (that cannot get RT_PRIO scheduling) might have a wakeup jitter of a lot more than a few ms, so for that type of process it would be a big improvement. And sometimes even RT_PRIO processes have big delays due to preempt_disable etc.

 3) The jitter will depend on the hardware, and other hardware might have better (or worse) IRQ jitter.

 4) If this patch gets accepted, it might show other kernel developers that IRQ jitter matters to us, and that in turn might improve the chances of getting IRQ jitter improvement patches in, in case someone else wants to help solving that problem.



My intension is not to discourage you, but it's better to have more care.
As one of the care, I think we can use extension of
'struct snd_rawmidi_status' to deliver some useful information to ALSA
rawmidi applications, or including other parameters to the frame structure.
But I don't have ideas about what information should be delivered,
sorry...
Well, the question is how much accuracy is wanted, and it's relatively
low for MIDI -- at least v1, which was defined many decades ago for a
slow serial line.

That said, the patch set was designed for providing the best-effort
timestamping in software, and that's supposed to be enough for normal
use cases.  If there is any device that is with the hardware
timestamping, in theory, we could provide the similar data stream in
the same format but maybe with a different timestamp type.

But actually I'd like to see some measurement how much we can improve
the timestamp accuracy by shifting the post office.  This may show
interesting numbers.

Sorry, I don't know the idiom "shifting the post office" and neither does the urban dictionary, so I have no idea what this means. :-)


Also, one thing to be more considered is the support for MIDI v2 in
future.  I haven't seen any development so far (and no device
available around), so I cannot comment on this much more, but it'd be
worth to take a quick look before defining the solid API/ABI.

I had a quick look at MIDI 2.0. It offers something called "Jitter reduction timestamps". After some searching I found that its resolution is 16 bit, and in units of 1/31250 seconds [1]. So the suggested timestamp format of secs + nsecs would suit us well for that case, I believe. When implemented, MIDI 2.0 jitter reduction timestamps would be another clock ID on top of the existing frame format (or a new frame format, if we prefer).

A midi 2.0 UMP (Universal Midi Packet) seems to be 4, 8, 12 or 16 bytes, excluding the timestamp. If we want to fit that format with the existing patch, we could increase the frame to 32 bytes so we can fit more data per packet. Do you think we should do that? Otherwise I think Patch v3 is ready for merging.

// David

[1] https://imitone.com/discover-midi/




[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