Re: [PATCH 1/2] usb: gadget: f_midi: refactor state machine
Felipe Ferreri Tonello wrote: >> Running status is feature. > >What do you mean by that? That this behavior is intended, and required. > I don't qualify writing a *wrong* MIDI-USB >packet because of a previous MIDI message as a feature. The MIDI Specification qualifies Running Status as a feature. Regards, Clemens -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] usb: gadget: f_midi: refactor state machine
Hi Clemens, On 22/12/15 17:10, Clemens Ladisch wrote: > Felipe F. Tonello wrote: >> This refactor includes the following: >> * Cleaner state machine code; > > It does not correctly handle system real time messages inserted between > the status and data bytes of other messages. True, thanks for pointing that out. I fixed that on next revision of this patch. > >> * Reset state if MIDI message parsed is non-conformant; > > Why? In a byte stream like "C1 C3 44", where the data byte of the first > message was lost, the reset would also drop the second message. True. That was fixed as well. > >> * Fixed bug when a conformant MIDI message was followed by a non-conformant >>causing the MIDI-USB message to use old temporary data (port->data[0..1]), >>thus packing a wrong MIDI-USB request. > > Running status is feature. What do you mean by that? I don't qualify writing a *wrong* MIDI-USB packet because of a previous MIDI message as a feature. For instance, try this MIDI message: "8A 54 24 00 40" It will be converted to MIDI-USB as "08 8A 54 24 08 8A 00 40" which is wrong. It should only be "08 8A 54 24" and ignore the "00 40" MIDI bytes. On every state byte the message should basically reset data[0..1] to zero overwriting previous data. This should also be true when a MIDI-USB packet is complete. Felipe 0x92698E6A.asc Description: application/pgp-keys
Re: [PATCH 1/2] usb: gadget: f_midi: refactor state machine
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Hi Balbi, On 22/12/15 17:49, Felipe Balbi wrote: > > Hi, > > "Felipe F. Tonello" writes: >> This refactor includes the following: * Cleaner state machine >> code; * Reset state if MIDI message parsed is non-conformant; * >> Fixed bug when a conformant MIDI message was followed by a >> non-conformant causing the MIDI-USB message to use old temporary >> data (port->data[0..1]), thus packing a wrong MIDI-USB request. > > we don't do more than one logical thing per patch. Please split > this up. Actually this patch has only one logical change, the state machine refactoring. But by doing it, those three items were a consequence. Felipe -BEGIN PGP SIGNATURE- Version: GnuPG v2 iQIcBAEBCAAGBQJWemc+AAoJEMxEtNCSaY5qpXwP/1B/sVsfapRtP4dw93YF6En5 w/ej9JatIyJIaXNxauCVzgRl9uuiXGyEqErRjJdodyvHar3yKvD9HEXE6MhowEp4 JMD/phN2v9Sdj1VxRf9Z0XDSWsg6huVhfQU47HBMRGiY8ezvEgir2bvg7dYbZMsv ACgIx8oh6N/AEHPq9GoLbEpXfJ58Pl564Sq/2o6wWsJQS06A+jp+JmqK4Y3eB5M5 18rmLW7lQLcZPO08Pf/c6BExWQLbzY/mT8KofwycvC9hWxYp9LPwJY4oMzOWROe8 AZS1ayRqlabG3qx3dPRcV4j6c6uROEfQY+HejCn+Zbi0CfVYtsI+xO5LkwnZMUyc 0qvy90h0PUQ2e37/wo5YnnVLK0ce1Gm6gY50iFXwqE69m55KHTufsIVX4eTUBfcj PtugPtTnKEsW171r/gHPYO9A+WKxycCvjPs9Wogsljly+NrRzgPMAI7Alx//4lwq QhwRWF1BkNOoCPEpHnVlLkcIhgdcAbbnvWxlcAVlLAQ/oYl6ShbQFv96y/2IWCVO Mwr7Ab8a/dnyJ/GWEQdpJUmfKGQbKNpM93H5yD4AQlmz4Hj620gzm3y2XNJY2bUt 8H37Q2VWtlcRy2UHjgBSeF0YKCvdDuDmZwRZbPpajkmbcYSGtJFDve1/L6bOtZSj 7nVfYIqvtIBWrDZ3PF9G =3uC5 -END PGP SIGNATURE- 0x92698E6A.asc Description: application/pgp-keys 0x92698E6A.asc.sig Description: PGP signature
Re: [PATCH 1/2] usb: gadget: f_midi: refactor state machine
Hi Clemens, On 22/12/15 17:10, Clemens Ladisch wrote: > Felipe F. Tonello wrote: >> This refactor includes the following: >> * Cleaner state machine code; > > It does not correctly handle system real time messages inserted between > the status and data bytes of other messages. True, thanks for pointing that out. I fixed that on next revision of this patch. > >> * Reset state if MIDI message parsed is non-conformant; > > Why? In a byte stream like "C1 C3 44", where the data byte of the first > message was lost, the reset would also drop the second message. True. That was fixed as well. > >> * Fixed bug when a conformant MIDI message was followed by a non-conformant >>causing the MIDI-USB message to use old temporary data (port->data[0..1]), >>thus packing a wrong MIDI-USB request. > > Running status is feature. What do you mean by that? I don't qualify writing a *wrong* MIDI-USB packet because of a previous MIDI message as a feature. For instance, try this MIDI message: "8A 54 24 00 40" It will be converted to MIDI-USB as "08 8A 54 24 08 8A 00 40" which is wrong. It should only be "08 8A 54 24" and ignore the "00 40" MIDI bytes. On every state byte the message should basically reset data[0..1] to zero overwriting previous data. This should also be true when a MIDI-USB packet is complete. Felipe 0x92698E6A.asc Description: application/pgp-keys
Re: [PATCH 1/2] usb: gadget: f_midi: refactor state machine
Felipe Ferreri Tonello wrote: >> Running status is feature. > >What do you mean by that? That this behavior is intended, and required. > I don't qualify writing a *wrong* MIDI-USB >packet because of a previous MIDI message as a feature. The MIDI Specification qualifies Running Status as a feature. Regards, Clemens -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] usb: gadget: f_midi: refactor state machine
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Hi Balbi, On 22/12/15 17:49, Felipe Balbi wrote: > > Hi, > > "Felipe F. Tonello"writes: >> This refactor includes the following: * Cleaner state machine >> code; * Reset state if MIDI message parsed is non-conformant; * >> Fixed bug when a conformant MIDI message was followed by a >> non-conformant causing the MIDI-USB message to use old temporary >> data (port->data[0..1]), thus packing a wrong MIDI-USB request. > > we don't do more than one logical thing per patch. Please split > this up. Actually this patch has only one logical change, the state machine refactoring. But by doing it, those three items were a consequence. Felipe -BEGIN PGP SIGNATURE- Version: GnuPG v2 iQIcBAEBCAAGBQJWemc+AAoJEMxEtNCSaY5qpXwP/1B/sVsfapRtP4dw93YF6En5 w/ej9JatIyJIaXNxauCVzgRl9uuiXGyEqErRjJdodyvHar3yKvD9HEXE6MhowEp4 JMD/phN2v9Sdj1VxRf9Z0XDSWsg6huVhfQU47HBMRGiY8ezvEgir2bvg7dYbZMsv ACgIx8oh6N/AEHPq9GoLbEpXfJ58Pl564Sq/2o6wWsJQS06A+jp+JmqK4Y3eB5M5 18rmLW7lQLcZPO08Pf/c6BExWQLbzY/mT8KofwycvC9hWxYp9LPwJY4oMzOWROe8 AZS1ayRqlabG3qx3dPRcV4j6c6uROEfQY+HejCn+Zbi0CfVYtsI+xO5LkwnZMUyc 0qvy90h0PUQ2e37/wo5YnnVLK0ce1Gm6gY50iFXwqE69m55KHTufsIVX4eTUBfcj PtugPtTnKEsW171r/gHPYO9A+WKxycCvjPs9Wogsljly+NrRzgPMAI7Alx//4lwq QhwRWF1BkNOoCPEpHnVlLkcIhgdcAbbnvWxlcAVlLAQ/oYl6ShbQFv96y/2IWCVO Mwr7Ab8a/dnyJ/GWEQdpJUmfKGQbKNpM93H5yD4AQlmz4Hj620gzm3y2XNJY2bUt 8H37Q2VWtlcRy2UHjgBSeF0YKCvdDuDmZwRZbPpajkmbcYSGtJFDve1/L6bOtZSj 7nVfYIqvtIBWrDZ3PF9G =3uC5 -END PGP SIGNATURE- 0x92698E6A.asc Description: application/pgp-keys 0x92698E6A.asc.sig Description: PGP signature
Re: [PATCH 1/2] usb: gadget: f_midi: refactor state machine
Hi, "Felipe F. Tonello" writes: > This refactor includes the following: > * Cleaner state machine code; > * Reset state if MIDI message parsed is non-conformant; > * Fixed bug when a conformant MIDI message was followed by a non-conformant >causing the MIDI-USB message to use old temporary data (port->data[0..1]), >thus packing a wrong MIDI-USB request. we don't do more than one logical thing per patch. Please split this up. -- balbi signature.asc Description: PGP signature
Re: [PATCH 1/2] usb: gadget: f_midi: refactor state machine
Felipe F. Tonello wrote: > This refactor includes the following: > * Cleaner state machine code; It does not correctly handle system real time messages inserted between the status and data bytes of other messages. > * Reset state if MIDI message parsed is non-conformant; Why? In a byte stream like "C1 C3 44", where the data byte of the first message was lost, the reset would also drop the second message. > * Fixed bug when a conformant MIDI message was followed by a non-conformant >causing the MIDI-USB message to use old temporary data (port->data[0..1]), >thus packing a wrong MIDI-USB request. Running status is feature. Regards, Clemens -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] usb: gadget: f_midi: refactor state machine
Hi, "Felipe F. Tonello"writes: > This refactor includes the following: > * Cleaner state machine code; > * Reset state if MIDI message parsed is non-conformant; > * Fixed bug when a conformant MIDI message was followed by a non-conformant >causing the MIDI-USB message to use old temporary data (port->data[0..1]), >thus packing a wrong MIDI-USB request. we don't do more than one logical thing per patch. Please split this up. -- balbi signature.asc Description: PGP signature
Re: [PATCH 1/2] usb: gadget: f_midi: refactor state machine
Felipe F. Tonello wrote: > This refactor includes the following: > * Cleaner state machine code; It does not correctly handle system real time messages inserted between the status and data bytes of other messages. > * Reset state if MIDI message parsed is non-conformant; Why? In a byte stream like "C1 C3 44", where the data byte of the first message was lost, the reset would also drop the second message. > * Fixed bug when a conformant MIDI message was followed by a non-conformant >causing the MIDI-USB message to use old temporary data (port->data[0..1]), >thus packing a wrong MIDI-USB request. Running status is feature. Regards, Clemens -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/