elastic_scale: added filter for elastic scaling

[schnoellm]

Hi there!

This elastic_scale filter allows to scale non-linearly scale video footage. Combined with linear scaling one can scale 4:3 video footage to 16:9, as it is for example used with GoPro's Supverview (https://gopro.com/help/articles/question_answer/What-is-SuperView).

Feel free to give feedback on improvements.

BR

[ddennedy]

This filter is a great idea! However, I get some bizarre results when I try to experiment with linearScaleArea or linearScaleFactor not zero as well as nonLinearScaleFactor larger than, say, 0.3. What further guidance can you give about using those? Does the mapping from the [0,1] range of these parameters need tuning to make it more usable?

[schnoellm]

Thanks, I really appreciate your feedback!

I think by experimenting yout've already got a good understanding of the parameters and their effects on the scaling ;-) However I also thought about restricting the parameters to a certain range, but this would then possibly limit users that might want to utilize exactly this "non-intended" way of working (e.g. "simply deleting the center part of the image" by having linearWidth > 0 and linearScaleFactor = 0)

Therefore I think we can leave the parameter ranges flexible for the user, as the default values already deliver a good starting point for further changes. One thing I could imagine, is to let the linearScaleFactor default to 0.7 or something, so the user immediately sees the effect when playing with the linearWidth parameter.

Yes, limiting nonLinearScaleFactor to [-0.2,0.2] probably makes much sense, to protect the users from missconfigurations.

[rrrapha]

I think the plugin should accept the parameter in the range [0.0,1.0] and map it to [-0.2,0.2] internally.

[schnoellm]

What do you think about using a range of [-1,1] and map it internally to the respective limits of [-0.2,0.2]? The reason why I prefer that range is because the filter would enable you to apply exactly 'zero' non-linearity (resp. linear scaling) with a value of 0, a certain sine based non-linearity with the value 1 and the exact opposit amount (also speaking of visible opposit) with a value of -1. Also it makes more sense to me when the 'zero' non-linearity is with value 0 instead of 0.5. What do you think about this approach?

[rrrapha]

It's not a free choice, it is how the interface is defined. ;) Applications like kdenlive or shotcut will assume a range of [0,1] and display a slider using that range.

This is from frei0r.h:

/**
 * The double type. The allowed range of values is [0, 1].
 */
typedef double f0r_param_double;

[schnoellm]

Ok, that's a valid point ;-) I will change that accordingly.

[ddennedy]

Obviously @rrrapha is correct, but also you do not need to strictly enforce the range in your plugin unless going outside would cause a crash. Best practice is to map at least 90% of the most usable actual range into [0, 1] where going beyond gives undefined results. Some users may try to experiment outside the range, which is fine (assuming their interface permits it)!

[schnoellm]

Just changed the input ranges for all parameters to fit into [0,1]. If the input exceeds this range the value is adjusted automatically. The only value currently mapped internally is nonLinearScaleFactor, which maps to [-0.2,0.2]. I think now it should be fine to be used in apps like kdenlive and shotcut later on.

[rrrapha]

I get a crash with certain combinations of parameters:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00000e017e4e9cad in ElasticScale::update (this=0xe006ef26000, time=0, out=0xe01748d2010, in=0xe01740d2010) at /home/rapha/frei0r/src/filter/elastic_scale/elastic_scale.cpp:104
104                             uint16_t cFold = uint8_t(in[lowerXPos] >> 8*i);
[Current thread is 1 (process 548290)]
(gdb) bt
#0  0x00000e017e4e9cad in ElasticScale::update (this=0xe006ef26000, time=0, out=0xe01748d2010, in=0xe01740d2010) at /home/rapha/frei0r/src/filter/elastic_scale/elastic_scale.cpp:104
#1  0x00000e017e4e992f in frei0r::filter::update (this=0xe006ef26000, time=0, out=0xe01748d2010, in1=0xe01740d2010, in2=0x0, in3=0x0) at /home/rapha/frei0r/include/frei0r.hpp:205
#2  0x00000e017e4e75cc in f0r_update2 (instance=0xe006ef26000, time=0, inframe1=0xe01740d2010, inframe2=0x0, inframe3=0x0, outframe=0xe01748d2010)
    at /home/rapha/frei0r/include/frei0r.hpp:336
#3  0x00000e017e4e7633 in f0r_update (instance=0xe006ef26000, time=0, inframe=0xe01740d2010, outframe=0xe01748d2010) at /home/rapha/frei0r/include/frei0r.hpp:347
#4  0x00000e0105eae0a7 in process_frei0r_item (service=<optimized out>, position=0, time=0, length=752, frame=0xe012604dc80, image=0xe00e6f62f58, width=0xe00e6f62f74, 
    height=<optimized out>) at frei0r_helper.c:40
#5  0x00000e0105ead3de in filter_get_image (frame=0xe012604dc80, image=0xe00e6f62f58, format=<optimized out>, width=0xe00e6f62f74, height=0xe00e6f62f70, writable=<optimized out>)
    at filter_frei0r.c:39
#6  0x00000e01117351d0 in mlt_frame_get_image (self=0xe012604dc80, buffer=0xe00e6f62f58, format=<optimized out>, width=<optimized out>, height=0xe00e6f62f70, writable=0)
    at mlt_frame.c:620
#7  0x00000e011174da97 in consumer_read_ahead_thread (arg=0xe014bf08000) at mlt_consumer.c:897
#8  0x00000dfe5d507f60 in RenderThread::run() ()
#9  0x00000e00ec36a342 in QThreadPrivate::start(void*) () from /usr/local/lib/libQt5Core.so.2.2
#10 0x00000e011f619fbe in _rthread_start (v=0xe01740d2010) at /usr/src/lib/librthread/rthread.c:96
#11 0x00000e007bba911b in __tfork_thread () at /usr/src/lib/libc/arch/amd64/sys/tfork_thread.S:75
#12 0x0000000000000000 in ?? ()
(gdb) info locals
cFold = 68
cCold = 31
i = 3
newValue = 0
lowerXPos = 46941436
higherXPos = 46941437
curPosDst = 56
rowIdx = 0
lowerWeight = 0.74863387644290924
higherWeight = 0.25136612355709076
colIdx = 56

[schnoellm]

@rrrapha the crash due to certain parameters should be fixed with the latest commit. Please also share the input parameters so I can try to reproduce the situation.

[rrrapha]

@schnoellm yes the crash is now gone ;)

..but there seems to be other problems: Can you try the following with an image size of 1290x849? ffmpeg -i in.jpg -vf "frei0r=elastic_scale:0.2|0.2|0.2|0.2" out.jpg The resulting image looks very funny here! With ohter sizes like 2048x1330 I'm seeing strange pixels in the first column.

[schnoellm]

@rrrapha the problem with frame sizes that are not multiples of 8 is with the associated padding in the frame buffer. Every row seems to consist out of a multiple of 8 pixels. Therefore there are some "hidden" pixels between consecutive rows within the frame buffer. Data being written to those "hidden" pixels is cut away afterwards and therefore each row seemed to be shifted by a certain amount to the left. This resulted in the funny effect of the distorted image.

It seems this padding is somehow described with the following section from frei0r.h

 * The following additional constraints must be honored:
 *   - The top-most line of a frame is stored first in memory.
 *   - A frame must be aligned to a 16 byte border in memory.
 *   - The width and height of a frame must be positive
 *   - The width and height of a frame must be integer multiples of 8

But it could also be interpreted that it would not allow images to have sizes other than integer multiples of 8. Probably this needs some more clarification in the API description to be interpreted properly. What do you think?

I also managed to fix the random colored pixels in the first column by limiting an intermediate scaling ratio to 0.

[rrrapha]

@schnoellm Thanks for the explanation, obviously width/height should be a multiple of 8. I think it doesn't hurt if other sizes work as well though ;) The filter now works fine for me (doesn't crash and looks reasonable).

Maybe this is pedantic, but frei0r.h also has this comment:

 * This funcition should not alter the parameters of the effect in any
 * way (\ref f0r_get_param_value should return the same values after a call
 * to \ref f0r_update as before the call).

Your code does change the parameter values if they are outside the range [0,1]. I'd be curious how @ddennedy thinks about this.

[schnoellm]

@rrrapha You are right, it is indeed a little pedantic. To be honest, you could have also mentioned that thing with the parameters earlier ;-) Nevertheless I've changed that in the code, so the external parameters are not modified and instead only the internal parameters are limited to [0,1] and mapped accordingly.

[rrrapha]

@schnoellm sorry, I didnt't notice this earlier.. It was meant as a question rather than an actual change request. (I can't speak for the project owners, I just thought the behaviour should match the API description)

[schnoellm]

@rrrapha it's ok ;-) As it was only a minor change in the code, it wasn't a big deal. I aggree, if there is the possibility to adhere to api constraints, it's valid to point out to them.