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

[Felipe Ferreri Tonello]

Hi Clemens,

On 12/08/16 12:38, Clemens Ladisch wrote:
> 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.

I see your point, you are right. Although, as we know, the underlying
driver won't send this byte anyway, but that is not for the application
to decide.

> 
>>>> +	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;

Right, that was not the original intent, so this code was actually
wrong. But I will remove it anyway based on your comment below.

> 
>>> 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.

True. So I will make sure to add this to the documentation.

-- 
Felipe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0x92698E6A.asc
Type: application/pgp-keys
Size: 7195 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20160812/ad01b86d/attachment.bin>