View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0000479Cinelerra-GG[All Projects] Bugpublic2020-07-20 15:462020-07-22 17:27
Reportersge Assigned ToPhyllisSmith  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version2020-04 
Target VersionFixed in Version2020-07 
Summary0000479: Wrong interpolation in cinelerra/affine.C, interp.h
DescriptionInterpolation (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 ReproduceI 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.
TagsNo tags attached.

Activities

PhyllisSmith

PhyllisSmith

2020-07-22 17:27

manager   ~0003830

Fix will be included in July 31 builds. Thanks for the help.
sge

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

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

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

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

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; \

Issue History

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