avfilter/vf_v360 interprets commands as relative rotation offsets

[Saul Baker]

Summary of the bug:

Use sendcmd to update the yaw,pitch or roll values of the v360 filter, contrary to the behaviour of other filters, the value sent is interpreted as an offset to apply to the current value rather than a value to set the property to.

This behaviour was switched in ​https://github.com/FFmpeg/FFmpeg/commit/d6e903b09b29fece449de37d5f8d6b20f3aa3333

This makes the filter stateful and hard to deal with, for example a smooth pan can no longer be achieved by applying lerp between two timestamps and instead spins the view wildly, generally position cannot be set to a known value without tracking an external copy of the rotation state.

The stateful 'relative rotation' approach seems at odds with the other filter behaviours that set the filter values rather than mutate them.

[Elon Musk]

This is intentional, otherwise you get gimbal lock.

[Saul Baker]

Replying to Elon Musk:

This is intentional, otherwise you get gimbal lock.

The rotations are always expressed though the api as euler angles, I'm puzzled how switching these from absolute assignment to relative offsets addresses gimbal lock as any triplet of values is still accessible - I'd ask you to reconsider as this is much less expressive.

For example the project ​https://github.com/dfaker/VR-reversal is completely unable to use the rotation values though the sendcmd api without changing to monitor a duplicate of the rotation state and calculate offsets, and even then smooth motion with lerp is not possible as offsets aren't expressible in the sendcmd command expressions.

[Michael Koch]

I can confirm that the example in chapter 2.110 of my book doesn't work any more.
​http://www.astro-electronic.de/FFmpeg_Book.pdf

[Elon Musk]

Gimbal lock is always there if absolute rotation is used, simple as that.

Perhaps you havent tried to rotate enough with v360 at all to experience this. Just rotate with 2 axis by near 180deg and than try to rotate by 3rd axis.

Please refrain from reopening or commenting on this issue if you have not anything new and relevant to add.

Also perhaps you want to solve your issues the wrong way from start.

[Saul Baker]

The main issue is that it breaks the assumptions of how property assignment works in a command to a filter - for every other sendcmd settable property they can be set to a value tested by passing that property to the filter directly and expect to see the filter behave as if that value were provided directly as a parameter during that command interval - and as a result interpolation functions can be evaluated in the sendcmd expressions to smooth or distribute value changes over time.

It may very well help with the issue of avoiding gimbal lock but that's at the expense of what you can express with the filter commands - this does regress the behaviour that the filer previously displayed by giving these properties unique behaviours - if the ability to specify a relative offset for these properties is required why not expose the adjustments of these values as separate rel_yaw, rel_roll, rel_roll as adding this unique behaviour to existing parameters breaks previous behaviour, developer expectations and the utility of sendcmd for setting these properties.

[Michael Koch]

Replying to Saul Baker:

It may very well help with the issue of avoiding gimbal lock but that's at the expense of what you can express with the filter commands - this does regress the behaviour that the filer previously displayed by giving these properties unique behaviours - if the ability to specify a relative offset for these properties is required why not expose the adjustments of these values as separate rel_yaw, rel_roll, rel_roll as adding this unique behaviour to existing parameters breaks previous behaviour, developer expectations and the utility of sendcmd for setting these properties.

I agree. Let the user decide if he prefers absolute or relative angles. If he chooses absolute angles, he must take care to avoid angles that lead to gimbal lock condition. I did make a lot of things with v360, but I did never run into this problem. I would always choose absolute angles. In my opinion, relative angles are useless.

[Saul Baker]

Replying to Elon Musk:

Please refrain from reopening or commenting on this issue if you have not anything new and relevant to add.

Also perhaps you want to solve your issues the wrong way from start.

I genuinely feel that the previous behaviour was more applicable to use cases of the filter, certainly in that repo example and Michael's book code it's the most direct and standardised expression of how to manipulate the filter - incremental stateful adjustment simply seems like the wrong model.

Replying to Michael Koch:

I would always choose absolute angles. In my opinion, relative angles are useless.

Yeah, I'm struggling to think of a situation where the relative would be useful, other than when you're seeing real-time output and want to nudge it manually but that doesn't really fit the ways the commands are used, at least through ffmpeg and sendcmd perhaps libav world is different?

Implementation wise: a mode or secondary property to target for relative rotation could use the flow in process_command as it stands at the moment whereas absolute rotation via mode or the default properties could reset the rot_quaternion (But not the filter rotation context) prior to ff_filter_process_command?

I appreciate that this is designed to make some use cases more pleasant and prevent the users getting into recoverable but confusing situations - but I think it's undeniable that there's a tension between this behaviour - how commands and users expect to update filter parameters, and what makes the most sense in defining scripted reprojection.

[Michael Koch]

Replying to Saul Baker:

I genuinely feel that the previous behaviour was more applicable to use cases of the filter, certainly in that repo example and Michael's book code it's the most direct and standardised expression of how to manipulate the filter - incremental stateful adjustment simply seems like the wrong model.

If it doesn't work any more in FFmpeg, you can do the same thing with DaVinci Resolve and the "KartaVR" plugin. You can define keyframes and set the rotation angles, and it does also interpolate the angles between the keyframes. I don't know if it works with the free version.

By the way, my approach for spherical stabilization with FFmpeg is now also broken. But that doesn't matter, because the spherical stabilizer in DaVinci Resolve is much better.

[Saul Baker]

After some testing it seems like replacing most of the functionality with a complete history of rotational sendcmds only has a small cumulative error offset compared to init with the same angles: it's not clear to me if periodic reinit is supposed to work here to minimise that.

But that still leaves the issue of interpolating rotations as a gap in the relative system I can't seem to close, I thought it may be possible to work it out in the current [expr] and divide with the 'N' variable populated during each sendcmd evaulation lerp(a,b,TI)/N but it's seemingly the start offset of the entire video with no way to work back to what the frame number was at the start of that specific eval interval - the timestamps don't seem to be of any use as one would need to work in frames rather than seconds.

[Michael Koch]

Replying to Saul Baker:

After some testing it seems like replacing most of the functionality with a complete history of rotational sendcmds only has a small cumulative error offset compared to init with the same angles:

I think that's only possible with one rotation axis. If you rotate around two or three axes simultaneously, then it's very complicated to calculate the required steps. I don't see any workaround for what was possible before the v360 filter was broken. It's a pity.

Michael

[Saul Baker]

Replying to Michael Koch:

I think that's only possible with one rotation axis. If you rotate around two or three axes simultaneously, then it's very complicated to calculate the required steps. I don't see any workaround for what was possible before the v360 filter was broken. It's a pity.

Michael

Separate [enter]s against all three interleaved in the same file seem to work quite happily, if jerkily, although I admit I've not gotten any more analytic with it than looking at a virtual lamppost and then flailing around wildly to get a good number of rotations to stack.

[Elon Musk]

I added reset_rot option, it basically allows to rotate absolutely if you set it after each rotation, have not checked how it will work with your sendcmd usage.

Also working on v360_opencl implementation for decent GPUs thus it will be much faster than CPU implementation.

[Saul Baker]

Replying to Elon Musk:

Also working on v360_opencl implementation for decent GPUs thus it will be much faster than CPU implementation.

Wonderful, the later optimizations you put into the CPU version have been a blazing transformation - I used to set the resolution way down low for using this interactively but the new release that's not even needed - can't wait to see how this runs accelerated!

I added reset_rot option, it basically allows to rotate absolutely if you set it after each rotation, have not checked how it will work with your sendcmd usage.

Thank you so much for your energies on this! To my eye perhaps inside process_command:

L:4965 s->reset_rot = 0;

is always assigned before it encounters

L:4971 if (s->reset_rot)

So it never reaches the insides of that if block? Hunting down a build now!

[Elon Musk]

The variable is set in function bellow, ff_filter_process_command.

[Saul Baker]

I half knew I'd not be catching anything real!

Super, I'll take a look in the next gyan build and see how it interleaves these commands when used in an [expr] sendcmd, thanks so much once again!

[Saul Baker]

Seems to work exceeding well for the sendcmd usecase, it doesn't even need to be stated on every line, as long as there exists a single expression that has an interval that covers all of the rotations it seems to be emitted and works like a dream:

5.022-28.996 [expr] v360 reset_rot '1';
5.022-5.022  [expr] v360 pitch 'lerp(0,-43.100,TI)', [expr] v360 yaw 'lerp(0,-12.266,TI)';
...
28.862-28.996  [expr] v360 pitch 'lerp(-13.754,-13.670,TI)', [expr] v360 yaw 'lerp(9.562,9.562,TI)';

As long as all changing rotations are pushed each frame by eval it all works as smooth as it ever was!

There are some interesting discontinuities when looking around the very edges of the 'frame' where the yaw is approaching the limits - it seems to be increasing what looks like the roll, I'll see how the various rotation orders affect it.

Great workaround, and thanks again Paul!

[Elon Musk]

That lerp with absolute rotation have nothing to do with this filter, iirc its happening in every app that use absolute rotations with lerp.

Read for example ​https://answers.unity.com/questions/717637/how-do-you-smoothly-transitionlerp-into-a-new-rota.html

[Saul Baker]

Replying to Elon Musk:

That lerp with absolute rotation have nothing to do with this filter, iirc its happening in every app that use absolute rotations with lerp.

Read for example ​https://answers.unity.com/questions/717637/how-do-you-smoothly-transitionlerp-into-a-new-rota.html

I'm more than happy to accept the slight weirdness implied by interpolating and forcing in the absolute Euler angles as long as it's possible and there's a good match between the interactive and sendcmd versions, which this gets to without a doubt.

I suppose an alternative to drop the absolute behaviour and have correct Quaternion controlled rotation and make this use pure relative would be to expose an additional property (alongside T,IT,N and the others) to the sendcmd expressions that allows it to work out how many frames are going to be emitted during the command interval so sendcmd scripts can use that as a divisor and emit the correct small slice of rotation each eval - but I appreciate that whereas time is easy frames are a minefield.

I suppose that'd be more pleasing for allowing the internals to be a more pure Qauternion based rotation, but as I say I'm more than delighted with this being functional with this flag in place!

[Michael Koch]

I did spend 2 hours testing the reset_rot option with an old example that did work before the rotations were changed from absolute to relative. In some cases it works, however in some other cases it fails and I don't yet understand why. I doubt that anybody will use it, because it's too complicated now.
I'm sure it would have been much better to keep the absolute rotations as they were, and add new options (with new names) for relative rotations. An open question is which kind of rotation has priority, if both are specified.
It's confusing to have an option with different behaviour in the command line / in sendcmd. No other filter has such behaviour. The sooner this is corrected, the better it is.

Michael

[Elon Musk]

Feel free to troll more, i will just block you.

[Saul Baker]

Landed in the mpv build now allowing for more complex testing of rotating multiple axes asynchronously.

The need to re-state the rotations for axes intended to be unmodified becomes quite troublesome, for example combining a:

yaw from -45 to 45 from 2-6s alongside a
pitch from -45 to 45 from 4-8s

Requires the pitch commands to be split up so that there are three separate command intervals for this movement, one for pure yaw, one for pitch and continuing the yaw movement, one for pitch alone but re-stating the now unmodified yaw.

In doing that I'm occasionally seeing quick flashes of some of rotations reset to 0 as intervals switch - I suspect it's either timestamp precision or the alignment of the timestamp in the command script and the presentation timestamp of the frame for which the interval expression is being evaludated.

I suppose the core questions are:

• Is it necessary to switch off reset_rot every command evaluation?
• Is it necessary to zero pitch,yaw,roll every evaluation while in reset_rot mode?

I'd tentatively suggest something like this might be a better way of handing the flag and rot resets:

static int process_command(AVFilterContext *ctx, const char *cmd, const char *args, char *res, int res_len, int flags)
{
    V360Context *s = ctx->priv;
    int ret;

    if (s->reset_rot) {
        reset_rot(s);
    } else {
        s->yaw = s->pitch = s->roll = 0.f;
    }

    ret = ff_filter_process_command(ctx, cmd, args, res, res_len, flags);
    if (ret < 0)
        return ret;

    return config_output(ctx->outputs[0]);
}

As it removes the side effects of resetting the untouched rotations and allows consistent setting of reset_rot mode once, perhaps I'm missing something, particularly in calling reset_rot before ff_filter_process_command?

-- On the plus side, that roll discontinuity I previously noted doesn't seem to be present in testing anymore, perhaps that was me just fat fingering the roll somewhere when I was limited to manually creating the command scripts.

[Elon Musk]

I changed option to always be active, and to be toggle only if set to -1.

[Saul Baker]

Replying to Elon Musk:

I changed option to always be active, and to be toggle only if set to -1.

That'll be a great convenience! Do you have any thoughts on limiting the zeroing of

s->yaw = s->pitch = s->roll = 0.f;

to only when reset_rot!=0?

[Elon Musk]

No

[Elon Musk]

No

[Elon Musk]

No

[Elon Musk]

No

[Saul Baker]

That seems pretty definitive, what was your reasoning?

It seems like zeroing these when reset_rot!=0 is an unintended hold-over from the relative mode which makes sense there as you need to know the change increment not the cumulative value, but here they're better left untouched as long as reset_rot!=0.

In fact it's a surprising side effect when reset_rot>0 and any other of the properties except yaw, pitch and roll are changed the view is suddenly clamped back to 0,0,0 - skipping the zeroing of those properties in the siutation where reset_rot!=0 would seem to make this reset_rot 'mode' the most predicatble for users and calling applications - and as a side effect make it fully consistent with the old absolute behaviour.

[Elon Musk]

You do not understand code at all. It uses quaternions, yaw, pitch and roll are fully irrelevant here.

[Balling]

Replying to Elon Musk:

No

4 times! When are going to update trac static resources on the server to fix this problem??

[Saul Baker]

Replying to Elon Musk:

You do not understand code at all. It uses quaternions, yaw, pitch and roll are fully irrelevant here.

Internally yes, but the interface exposed to the users is Euler angles, if that has surprising behaviour or has usability issues (such as jumping to origin when the user's command script stops setting rotations when reset_rot!=0) it doesn't matter how excellent a job you've done on matrix multiplication.

There is nothing wrong with the internals of the implementation, the concern is purely in the surprising or hard to use elements of the interface for users, specifically command processing - and the remaining issue with that is the zeroing of all rotations on any execution of process_command, that makes sense in a relative mode but not when reset_rot is set.

[Elon Musk]

quaternion matrix is changed only when yaw/pitch/roll are not 0

I would like to educated uneducated, but this is neither time, nor place, nor way...

[Saul Baker]

Pitch reset on subsequent yaw command

[Saul Baker]

Replying to Elon Musk:

quaternion matrix is changed only when yaw/pitch/roll are not 0

Perfectly correct that's precisely the issue, it's being reset to origin when reset_rot is set, and then the yaw,pitch,roll are being set to zero, and then it's applying the commands to update the yaw,pitch,roll with new values, if none of the commands in that command interval are changing the yaw/pitch/roll they stay at zero and as you rightly say: no update to the quaternion and it stays in it's reset state pointing at origin for that axis.

For example with the command sequence:

0.0000-1.2622 [expr] v360@1 reset_rot '1', v360@1 pitch 'lerp(-15.0000,-15.0000,TI)';
1.2622-2.8044 [expr] v360@1 reset_rot '1', v360@1 pitch 'lerp(-15.0000,15.0000,TI)';
0.0000-3.6991 [expr] v360@1 reset_rot '1', v360@1 yaw 'lerp(-20.0000,-20.0000,TI)';
3.6991-4.9172 [expr] v360@1 reset_rot '1', v360@1 yaw 'lerp(-20.0000,20.0000,TI)';

Produces the attached 'Pitch reset on subsequent yaw command' example where on execute of the yaw commands that don't mention the pitch the pitch is reset to origin - however if the yaw/pitch/roll were not zeroed when reset_rot!=0 the pitch would have retained its prior value correctly staying rotated to the previously specified command's angle of 15degrees upwards even as it was lerping through the yaw sweep.

[Saul Baker]

I'd also note it's not limited to commanding the rotation angles, anything that runs process_command is setting the yaw/pitch/roll state to zero and then not moving the quaternion from origin when reset_rot!=0, snapping the view back to 0,0,0 for example a pan up followed by a small zoom:

0.0000-1.2622 [expr] v360@3 reset_rot '1', v360@3 pitch 'lerp(-15.0000,-15.0000,TI)';
1.2622-2.8044 [expr] v360@3 reset_rot '1', v360@3 pitch 'lerp(-15.0000,15.0000,TI)';
4.9172 [enter] v360@3 reset_rot '1', v360@3 d_fov 95;

Also resets the view to origin as soon as 4.9172 is entered, that and the above issue is what the question around limiting the zeroing of the user supplied rotations s->yaw = s->pitch = s->roll = 0.f; to only when reset_rot!=0 was addressing - I humbly think it's a better, more useful and predictable behaviour.

[Michael Koch]

Replying to Saul Baker:

I'd also note it's not limited to commanding the rotation angles, anything that runs process_command is setting the yaw/pitch/roll state to zero and then not moving the quaternion from origin when reset_rot!=0, snapping the view back to 0,0,0 for example a pan up followed by a small zoom:

0.0000-1.2622 [expr] v360@3 reset_rot '1', v360@3 pitch 'lerp(-15.0000,-15.0000,TI)';

I'm surprised that this line works for you. I get an error message when I omit [expr] before the second v360.

Michael

[Saul Baker]

Replying to Michael Koch:

Replying to Saul Baker:

I'd also note it's not limited to commanding the rotation angles, anything that runs process_command is setting the yaw/pitch/roll state to zero and then not moving the quaternion from origin when reset_rot!=0, snapping the view back to 0,0,0 for example a pan up followed by a small zoom:

0.0000-1.2622 [expr] v360@3 reset_rot '1', v360@3 pitch 'lerp(-15.0000,-15.0000,TI)';

I'm surprised that this line works for you. I get an error message when I omit [expr] before the second v360.

Michael

I merged the lines when copying it over so may have messed it up: but believe that was functional, your way would be my natural suspicion for correctness though.

[Michael Koch]

I changed option to always be active, and to be toggle only if set to -1.

Paul,

What you are trying to implement is rotational velocity for the yaw, pitch and roll axes. I suggest not to call it "relative rotation" but instead "rotational velocity". Don't specify it as an angle, but instead as angle per second, so that it becomes independant of framerate.

At the moment we have this situation:
In the command line it's possible to specify yaw, pitch and roll angles, but it's not possible to specify rotational velocity.
In sendcmd it's possible to specify rotational velocity (in the unit angle per frame), and specifying absolute yaw, pitch and roll angles is also possible with the "reset_rot=-1" workaround, but in my opinion this is complicated not user-friendly.

My suggestion:
We should clearly distinguish between angles and rotational velocities. Use different options for these different things. Then both of them could be specified in the command line and also both of them could be specified via sendcmd. That would be much clearer and easy to use.

Michael