Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix processing 8-bit RGB images #31

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

Karry
Copy link
Contributor

@Karry Karry commented Dec 3, 2015

Hi, I am creating tools for building timelapse videos. Now I am trying to create tool that reads JPEG images (by ImageMagic) and stabilize it with vidstab lib. I found some problems with processing RGB (8bit depth) images that ends with SIGSEGV. Read commit messages for more information...

@TouchMonkey
Copy link

Just a heads up, in vidstabdefines.h you fixed the PIXN macro but the same mistake is also in PIXELN right above it. PIXELN is used if floating point is enabled instead of fixed point, but they should probably both be fixed.

@Karry
Copy link
Contributor Author

Karry commented Dec 15, 2020

Hi @georgmartius . I updated dependencies on my timelapse project to make it buildable again... And this bug is still reproducible with vid.stab master - it is crashing still. Are you interested for the fix? Should I rebase it or update it somehow? Problem is that I don't remember exact details about the patch :-D

@georgmartius
Copy link
Owner

Hi, Sorry for ignoring your patch at the time.
About the boundary checks: The fields are constructed such that they can never leave the image. So we don't need these checks. Remember this is a very core function that is called thousands of times, see we want to be fast here.
Can you please check again and only have the fix for showing the fields in RGB.

Field is sometimes located outside of the frame after transformation.
It can cause crash in compareSubImg_thr_sse2 function.
@Karry
Copy link
Contributor Author

Karry commented Dec 15, 2020

Ok, I just rebased my branch on current upstream master. I tried to build it without last commit. And it crashed....

Thread 1 "timelapse_stabi" received signal SIGSEGV, Segmentation fault.
0x00007ffff7fb3f77 in interpolateN (rv=0x7fffe121da2a "\017\021\020\017\021\020", x=323646068, y=214368265, 
    img=0x7fffe121e010 "@fq@fq@fq@fq?ep?ep>do>doAgr@fq?ep>do=cn=cn=cn=cn>bp>bp=cp=cp<dp;co;co:bn:dp9co;co;co:bn:bn;co;co;co;co:bn:bn:bn9am:bn:bn:bn:bn:bn;co;co;co;co;co9am9am9am:bn:bn;co<bo<bo;an<bo<bm<bm=cn<bm<bm<bm<bm<bm=c"..., 
    img_linesize=14820, width=4940, height=3272, N=3 '\003', channel=0 '\000', def=15 '\017') at /home/karry/data/cecko/vidstab/src/transformfixedpoint.c:246
246         short v1 = PIXN(img, img_linesize, ix_c, iy_c, N, channel);
(gdb) bt
#0  0x00007ffff7fb3f77 in interpolateN (rv=0x7fffe121da2a "\017\021\020\017\021\020", x=323646068, y=214368265, 
    img=0x7fffe121e010 "@fq@fq@fq@fq?ep?ep>do>doAgr@fq?ep>do=cn=cn=cn=cn>bp>bp=cp=cp<dp;co;co:bn:dp9co;co;co:bn:bn;co;co;co;co:bn:bn:bn9am:bn:bn:bn:bn:bn;co;co;co;co;co9am9am9am:bn:bn;co<bo<bo;an<bo<bm<bm=cn<bm<bm<bm<bm<bm=c"..., 
    img_linesize=14820, width=4940, height=3272, N=3 '\003', channel=0 '\000', def=15 '\017') at /home/karry/data/cecko/vidstab/src/transformfixedpoint.c:246
#1  0x00007ffff7fb4357 in transformPacked (td=0x555555592620, t=...) at /home/karry/data/cecko/vidstab/src/transformfixedpoint.c:307
#2  0x00007ffff7fb0d08 in vsDoTransform (td=0x555555592620, t=...) at /home/karry/data/cecko/vidstab/src/transform.c:176
#3  0x00007ffff7f7f28a in timelapse::PipelineStabTransform::onInputImg (this=0x5555555925f0, info=..., image=<incomplete type>) at /home/karry/data/cecko/QT/TimeLapse/src/pipeline_stab.cpp:617
...
(gdb) print ix_f
$2 = 4938
(gdb) print iy_f
$3 = 3271
(gdb) print width
$4 = 4940
(gdb) print height
$5 = 3272
(gdb) print iy_c
$6 = 3272

iy_c definitely is outside image boundary...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants