[RFC 4/11] Linux FF: Linux effect parameter: periodic effect's phase

[Elias Vanderstuyft]

a)  Periodic phase is not implemented conform most efficient
implementations of linux FF-devices:
    phase should be related to time, not to angle:
    - HW interface example: In /drivers/input/joystick/iforce/iforce-ff.c :
        make_period_modifier(...): "iforce_send_packet(iforce,
FF_CMD_PERIOD, data)" where data[4] = HI(phase);
        make_core(...): "iforce_send_packet(iforce, FF_CMD_EFFECT,
data)" where data[4] = HI(duration);
        => Conclusion: phase likely to be related to time, not to angle
    - SW kernel interface example, which is going to be used for all
memless FF devices in the future:
        (http://permalink.gmane.org/gmane.linux.kernel/1620326) in
ff-memless-next.c:149 :
        mlnxeff->playback_time = effect->u.periodic.phase
    - SW interface example: SDL2-2.0.1/src/haptic/linux/SDL_syshaptic.c:640

    There is also written on /include/uapi/linux/input.h:1058 :
        "@phase: 'horizontal' shift"
        where 'horizontal' most likely means time dimension, not phase
of the waveform.
    => To be sure, I asked this at
http://www.mail-archive.com/linux-input@vger.kernel.org/msg08459.html,
        but didn't receive any answer yet.

    So change the following in dinput/effect_linuxinput.c:259 :
        tsp->lOffset = (This->effect.u.periodic.offset / 33) * 10;
        tsp->dwPhase = (This->effect.u.periodic.phase / 33) * 36;
        tsp->dwPeriod = (This->effect.u.periodic.period * 1000);
    to :
        tsp->lOffset = (This->effect.u.periodic.offset / 33) * 10;
        if (!This->effect.u.periodic.period) {
            /* Avoid division by zero period, set tsp->dwPhase
arbitrarily to zero. */
            tsp->dwPhase = 0;
        } else
            tsp->dwPhase = (((int)This->effect.u.periodic.phase *
36000) / This->effect.u.periodic.period) % 36000;
        tsp->dwPeriod = (This->effect.u.periodic.period * 1000);

    and change the following in dinput/effect_linuxinput.c:486 :
        This->effect.u.periodic.offset = (tsp->lOffset / 10) * 32;
        This->effect.u.periodic.phase = (tsp->dwPhase / 9) * 8; /* ==
(/ 36 * 32) */
        This->effect.u.periodic.period = tsp->dwPeriod / 1000;
    to :
        This->effect.u.periodic.offset = (tsp->lOffset / 10) * 32;
        This->effect.u.periodic.period = tsp->dwPeriod / 1000;
        This->effect.u.periodic.phase = ((int)(tsp->dwPhase % 36000) *
This->effect.u.periodic.period) / 36000;


Elias
-------------- next part --------------
/////////////////////////////    Linux effect parameter: periodic effect's phase    /////////////////////////////

a)  Periodic phase is not implemented conform most efficient implementations of linux FF-devices:
    phase should be related to time, not to angle:
    - HW interface example: In /drivers/input/joystick/iforce/iforce-ff.c :
        make_period_modifier(...): "iforce_send_packet(iforce, FF_CMD_PERIOD, data)" where data[4] = HI(phase);
        make_core(...): "iforce_send_packet(iforce, FF_CMD_EFFECT, data)" where data[4] = HI(duration);
        => Conclusion: phase likely to be related to time, not to angle
    - SW kernel interface example, which is going to be used for all memless FF devices in the future:
        (http://permalink.gmane.org/gmane.linux.kernel/1620326) in ff-memless-next.c:149 :
        mlnxeff->playback_time = effect->u.periodic.phase
    - SW interface example: SDL2-2.0.1/src/haptic/linux/SDL_syshaptic.c:640
    
    There is also written on /include/uapi/linux/input.h:1058 :
        "@phase: 'horizontal' shift"
        where 'horizontal' most likely means time dimension, not phase of the waveform.
    => To be sure, I asked this at http://www.mail-archive.com/linux-input@vger.kernel.org/msg08459.html,
        but didn't receive any answer yet.
    
    So change the following in dinput/effect_linuxinput.c:259 :
        tsp->lOffset = (This->effect.u.periodic.offset / 33) * 10;
        tsp->dwPhase = (This->effect.u.periodic.phase / 33) * 36;
        tsp->dwPeriod = (This->effect.u.periodic.period * 1000);
    to :
        tsp->lOffset = (This->effect.u.periodic.offset / 33) * 10;
        if (!This->effect.u.periodic.period) {
            /* Avoid division by zero period, set tsp->dwPhase arbitrarily to zero. */
            tsp->dwPhase = 0;
        } else
            tsp->dwPhase = (((int)This->effect.u.periodic.phase * 36000) / This->effect.u.periodic.period) % 36000;
        tsp->dwPeriod = (This->effect.u.periodic.period * 1000);
    
    and change the following in dinput/effect_linuxinput.c:486 :
        This->effect.u.periodic.offset = (tsp->lOffset / 10) * 32;
        This->effect.u.periodic.phase = (tsp->dwPhase / 9) * 8; /* == (/ 36 * 32) */
        This->effect.u.periodic.period = tsp->dwPeriod / 1000;
    to :
        This->effect.u.periodic.offset = (tsp->lOffset / 10) * 32;
        This->effect.u.periodic.period = tsp->dwPeriod / 1000;
        This->effect.u.periodic.phase = ((int)(tsp->dwPhase % 36000) * This->effect.u.periodic.period) / 36000;