0000479: Wrong interpolation in cinelerra/affine.C, interp.h - Cinelerra-GG MantisBT

ID	Project	Category	View Status	Date Submitted	Last Update

0000479	Cinelerra-GG	[All Projects] Bug	public	2020-07-20 15:46	2020-07-22 17:27

Reporter	sge	Assigned To	PhyllisSmith
Priority	normal	Severity	minor	Reproducibility	always
Status	closed	Resolution	fixed
Product Version	2020-04
Target Version		Fixed in Version	2020-07

Summary	0000479: Wrong interpolation in cinelerra/affine.C, interp.h
Description	Interpolation (bilinear and bicubic) used in AffineEngine is wrong: the coordinates and displacements of the interpolated pixels are calculated incorrectly. A consequence example: rotation to 0.000000 degrees leads to the displacement of the frame 1 pixel to the right and 1 pixel down. The right and bottom transformation region limits are calculated incorrectly, they are 1 pixel less than necessary. In AffineEngine::init_packages() y1 is calculated incorrectly for the 1st package (when i=0).
Steps To Reproduce	I reproduced this problem while debugging the Motion plugin. I tried to stabilize a test video consisting of a series of frames which differ from each other in noise only. After dumping (via write_ppm()) a frame before and after its rotation to 0.000000 degrees, the result was displaced 1 pixel right-down. To reproduce the problem in such a way, it is necessary to modify the plugin code. However, I can make such a debugging patch if the bug remains not obvious after looking at the attached correcting patch.
Tags	No tags attached.

PhyllisSmith 2020-07-22 17:27 manager ~0003830	Fix will be included in July 31 builds. Thanks for the help.

sge 2020-07-22 12:33 reporter ~0003829	I have looked on the diffs in GIT, which are relevant to my suggested patch - everything OK, thank you. I think, you can close this issue.

PhyllisSmith 2020-07-21 17:43 manager ~0003822 Last edited: 2020-07-21 17:44 View 2 revisions	@sge Mods for affine.C and interp.h have been checked into GIT that include your patch bug fix, specifically: int y1 = out_y, y2 = out_y, npkgs = get_total_packages(); Great debugging, research, and analysis on your part to find this bug! We can tell it took a lot of your time to include providing the test case. We just now used the test case to show that the checked in mod seems to cover the Motion problem. As you initially noted yesterday, affine is used in various places so when GG was looking into this yesterday, he started by using the Perspective plugin as it is the easiest to work with. So he spent the day checking a lot of different boundary cases too and found some other problems in interp.h to include the ones you pointed out so all got fixed (hopefully). Again, we want to thank you for diligently pursuing these problems so we can continue to improve the code and the test case really helped a lot. Oh, and I am supposed to mention that GG did not put in the patch improved/corrected comment changes and stylistic changes in order to minimize the modset. This is just to ensure there are no unexpected consequences for compiling all of the different (and some very old 32-bit) distros.

sge 2020-07-21 15:25 reporter ~0003821	I have forgotten to mention: the affine bug appears under the CPU processing. I expect that under the OpenGL (GPU) processing it may not come out. The 'Motion' plugin does not use OpenGL, therefore the bug was visible. I can suggest the following sequence to reproduce the bug appearance. Take and unpack the attached file affine-test.tar.gz. It contains a Cinelerra project file and a short synthetic video just for the test. 1. Start cin. 2. Load project 't1.xml'. The asset 't1.m4v' must be in the same directory. Preferences -> Playback -> Play every frame Preferences -> Playback -> Scaling equation: BiCubic/BiCubic FFMPEG status: 'Try FFMpeg first'. Set cursor to the beginning of the timeline. 3. There must be 2 seconds of a synthetic video in the timeline with the attached 'Motion' plugin. Open plugin controls. Track translation, Track rotation, Draw vectors - all ON Action: Do Nothing, Calculation: Don't Calculate Tracking file: /tmp/m Other controls should have some reasonable values. The translation block defined should enclose all the geometry shapes. 4. Remove the file /tmp/m if it eventually exists. Calculation: Save coords to tracking file Play the project. Playback will be slow due to plugin. Although the picture is almost static, the frame showing rotation will rotate clockwise. If you look at the contents of /tmp/m, it will show rotation angles of 0.24 deg or something like this. So is the appearance of the bug. 5. You can now switch controls: Calculation: Load coords from tracking file Rewind to the beginning of the timeline Action: Stabilize Subpixel Draw vectors - OFF Render project to some output file. You will notice a strong counterclockwise rotation of the rendered picture. 6. Quit Cinelerra (don't save anything). 7. Unpack Cinelerra sources, apply the affine/interp patch, build and start this patched version of cin. 8. Repeat items 2 and 3. Remove the file /tmp/m from the previuos run. 9. Calculation: Save coords to tracking file. Play the project. Now the rectangular frame will show almost no rotation. All the angles in /tmp/m will be very small, about 0.002 deg. 10. Switch controls: Calculation: Load coords from tracking file Rewind to the beginning of the timeline Action: Stabilize Subpixel Draw vectors - OFF Render the project. Now you will notice that most of the video does not rotate any more. A small rotation can be observed only during the last 7-8 frames, where the source video is rotating itself (such a rotation is poorly compensated). But the whole region without rotation is now OK. The bug appearance and the effect of the affine.C correction is demonstrated. affine-test.tar.gz (734,516 bytes)

PhyllisSmith 2020-07-20 16:37 manager ~0003816	Interesting! GG will be looking at this today and even though I do not understand it, most likely the patch will be clear to him. Thanks for reporting and thanks for the patch -- it is very much appreciated.

sge 2020-07-20 15:46 reporter	cinelerra-5.1-affineinterp.diff (3,663 bytes) diff -Naur cinelerra-5.1.orig/cinelerra/affine.C cinelerra-5.1/cinelerra/affine.C --- cinelerra-5.1.orig/cinelerra/affine.C 2019-06-16 01:47:50.000000000 +0700 +++ cinelerra-5.1/cinelerra/affine.C 2020-07-20 20:27:35.790041180 +0700 @@ -185,7 +185,7 @@ double w; w = values[2][0] * x + values[2][1] * y + values[2][2]; - w = !w ? 1 : 1/w; + w = ((w == 0.0) ? 1.0 : 1.0/w); newx = (values[0][0] x + values[0][1] * y + values[0][2]) * w; newy = (values[1][0] x + values[1][1] * y + values[1][2]) * w; @@ -242,8 +242,8 @@ AffinePackage pkg = (AffinePackage)package; int min_in_x = server->in_x; int min_in_y = server->in_y; - int max_in_x = server->in_x + server->in_w - 1; - int max_in_y = server->in_y + server->in_h - 1; + int max_in_x = server->in_x + server->in_w; + int max_in_y = server->in_y + server->in_h; // printf("AffineUnit::process_package %d %d %d %d %d\n", @@ -690,10 +690,10 @@ void AffineEngine::init_packages() { - int y1 = 0, npkgs = get_total_packages(); + int y1 = out_y, y2 = out_y, npkgs = get_total_packages(); for( int i=0; i<npkgs; ) { AffinePackage package = (AffinePackage)get_package(i); - int y2 = out_y + (out_h * ++i / npkgs); + y2 = out_y + (out_h * ++i / npkgs); package->y1 = y1; package->y2 = y2; y1 = y2; } } @@ -829,7 +829,7 @@ x4 = ((in_pivot_x - in_x) - sin(angle4) * radius4) * 100 / in_w; y4 = ((in_pivot_y - in_y) + cos(angle4) * radius4) * 100 / in_h; -// printf("AffineEngine::rotate angle=%f\n", +// printf("AffineEngine::rotate use_opengl=%d angle=%f\n", use_opengl, // angle); // @@ -841,10 +841,10 @@ // radius1, radius2, radius3, radius4); // // printf(" x1=%f y1=%f x2=%f y2=%f x3=%f y3=%f x4=%f y4=%f\n", -// x1 * w / 100, y1 * h / 100, -// x2 * w / 100, y2 * h / 100, -// x3 * w / 100, y3 * h / 100, -// x4 * w / 100, y4 * h / 100); +// x1 * in_w / 100, y1 * in_h / 100, +// x2 * in_w / 100, y2 * in_h / 100, +// x3 * in_w / 100, y3 * in_h / 100, +// x4 * in_w / 100, y4 * in_h / 100); if(use_opengl) { set_package_count(1); diff -Naur cinelerra-5.1.orig/cinelerra/interp.h cinelerra-5.1/cinelerra/interp.h --- cinelerra-5.1.orig/cinelerra/interp.h 2019-06-16 01:47:50.000000000 +0700 +++ cinelerra-5.1/cinelerra/interp.h 2020-07-20 20:27:54.880041556 +0700 @@ -51,7 +51,7 @@ #define nearest_SETUP(typ, components, tx, ty) \ - int itx = (tx), ity = (ty), in_comps = (components); \ + int itx = (int)(tx), ity = (int)(ty), in_comps = (components); \ int c0 = itx+0, r0 = ity+0; \ typ r0p = r0>=in_min_y && r0<in_max_y ? ((typ)interp_rows)[r0] : 0 @@ -64,10 +64,8 @@ #define bi_linear_SETUP(typ, components, tx, ty) \ - float dx = (tx)-0.5, dy = (ty)-0.5; \ - int itx = dx, ity = dy, in_comps = (components); \ - if( (dx -= itx) < 0 ) dx += 1; \ - if( (dy -= ity) < 0 ) dy += 1; \ + int itx = (int)(tx), ity = (int)(ty), in_comps = (components); \ + float dx = (tx)-itx, dy = (ty)-ity; \ int c0 = itx+0, c1 = itx+1, r0 = ity+0, r1 = ity+1; \ typ r0p = r0>=in_min_y && r0<in_max_y ? ((typ*)interp_rows)[r0] : 0; \ typ r1p = r1>=in_min_y && r1<in_max_y ? ((typ*)interp_rows)[r1] : 0 @@ -83,10 +81,8 @@ #define bi_cubic_SETUP(typ, components, tx, ty) \ - float dx = (tx)-0.5, dy = (ty)-0.5; \ - int itx = dx, ity = dy, in_comps = (components); \ - if( (dx -= itx) < 0 ) dx += 1; \ - if( (dy -= ity) < 0 ) dy += 1; \ + int itx = (int)(tx), ity = (int)(ty), in_comps = (components); \ + float dx = (tx)-itx, dy = (ty)-ity; \ int cp = itx-1, c0 = itx+0, c1 = itx+1, c2 = itx+2; \ int rp = ity-1, r0 = ity+0, r1 = ity+1, r2 = ity+2; \ typ rpp = rp>=in_min_y && rp<in_max_y ? ((typ**)interp_rows)[rp] : 0; \ cinelerra-5.1-affineinterp.diff (3,663 bytes)

Date Modified	Username	Field	Change
2020-07-20 15:46	sge	New Issue
2020-07-20 15:46	sge	File Added: cinelerra-5.1-affineinterp.diff
2020-07-20 16:37	PhyllisSmith	Assigned To	=> PhyllisSmith
2020-07-20 16:37	PhyllisSmith	Status	new => acknowledged
2020-07-20 16:37	PhyllisSmith	Note Added: 0003816
2020-07-21 15:25	sge	File Added: affine-test.tar.gz
2020-07-21 15:25	sge	Note Added: 0003821
2020-07-21 17:43	PhyllisSmith	Note Added: 0003822
2020-07-21 17:44	PhyllisSmith	Note Edited: 0003822	View Revisions
2020-07-22 12:33	sge	Note Added: 0003829
2020-07-22 17:27	PhyllisSmith	Status	acknowledged => closed
2020-07-22 17:27	PhyllisSmith	Resolution	open => fixed
2020-07-22 17:27	PhyllisSmith	Fixed in Version	=> 2020-07
2020-07-22 17:27	PhyllisSmith	Note Added: 0003830