[alsa-devel] [PATCH] amidi: add delay option

[Clemens Ladisch]

Felipe Ferreri Tonello wrote:
> On 12/08/16 08:10, Clemens Ladisch wrote:
>> Felipe F. Tonello wrote:
>>> +	case 0xf4:
>>> +	case 0xf5:
>>> +		return 0; /* ignore */
>>> +	case 0xf7:
>>> +		return 0; /* ignore */
>>
>> Silent data loss is bad.
>
> What do you mean by that?

When you want to send the byte F5, this code will silently drop it.

>>> +	case 0x00 ... 0x7f:
>>> +		return rstate_len ?: rstate_len - 1;
>>
>> rstate_len - 1 == -1
>
> ?
>
> That's why there is this condition, if rstate_len is zero, than it
> returns 0.

There is not need to use rstate_len when its value is already known and
does not affect the computation; it could just be written as:

        return rstate_len ?: -1;

>> The most robust way to find message boundaries probably would be a state
>> machine.  However, there is actually no requirement to find all message
>> boundaries; the _actual_ requirement is to insert pauses between SysEx
>> messages.  So don't bother to try to parse everything; just search for
>> F0 or F7.
>
> Well, I tough it would be useful to work in any type of MIDI message not
> just SysEx.

But there is no _actual_ use case, where somebody actually needed it.
And as shown by all those problems, handling all types of messages makes
the code too complex.


Regards,
Clemens