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

Godot crashes when importing 1x1 pixel JPEG image using YCbCr and 2x2 sampling factor #42363

Closed
EdThePro101 opened this issue Sep 27, 2020 · 6 comments · Fixed by #43441 · May be fixed by richgel999/jpeg-compressor#16
Closed

Comments

@EdThePro101
Copy link

Godot version:
Godot 3.2.3

OS/device including version:

  • Device: Acer Aspire A315-33
  • OS: Windows 10 64-bit
  • CPU: Intel(R) Celeron(R) N3060 @ 1.60 GHz
  • GPU: Intel HD Graphics 400
  • RAM: 4GB

Issue description:
I was following an answer on the Godot forum about making a coloured rectangle, and using Microsoft Paint, I made a white 1x1 pixel JPEG image and placed it in my project's .\Images\ directory. When I opened Godot to rescan for assets, Godot crashed as it was importing .\Images\Background.jpg.

Steps to reproduce:

  1. Create an empty Godot project.
  2. In the small File Explorer view on the bottom left, right-click on res:// and create a folder called Scenes.
  3. Open MS Paint.
  4. Go to File -> Preferences .
  5. Change both width and height to 1 pixel.
  6. Tap on the white colour in the colour palette.
  7. Tap on the single pixel (may need to zoom in).
  8. Go to File -> Save As... -> JPEG image.
  9. Go to the project's directory and make a new folder called Images.
  10. Save the JPEG image as Background.jpeg or Background.jpg.
  11. Open Godot and let Godot scan the project's assets.
  12. The crash occurs when scanning Background.jpeg.

Minimal reproduction project:
I've added the original project.

ForEx Simulator.zip

@Calinou
Copy link
Member

Calinou commented Sep 27, 2020

I can't reproduce this on Godot 3.2.3 with JPEGs generated by ImageMagick (convert -size 1x1 xc:"#fff" 1x1.jpg). However, I can reproduce this with your MRP:

godot.x11.tools.64.llvm: thirdparty/jpeg-compressor/jpgd.h:278: int jpgd::jpeg_decoder::check_sample_buf_ofs(int) const: Assertion `ofs >= 0' failed.

My minimal reproduction project: small_jpeg_import_crash.zip

file information for your MRP:

Background.jpg: JPEG image data, JFIF standard 1.01, resolution (DPI), density 96x96, segment length 16, baseline, precision 8, 1x1, components 3

file information for my MRP:

1x1.jpg: JPEG image data, JFIF standard 1.01, aspect ratio, density 1x1, segment length 16, baseline, precision 8, 1x1, components 1
1x2.jpg: JPEG image data, JFIF standard 1.01, aspect ratio, density 1x1, segment length 16, baseline, precision 8, 1x2, components 1
2x2.jpg: JPEG image data, JFIF standard 1.01, aspect ratio, density 1x1, segment length 16, baseline, precision 8, 2x2, components 1
2x3.jpg: JPEG image data, JFIF standard 1.01, aspect ratio, density 1x1, segment length 16, baseline, precision 8, 2x3, components 1
2x4.jpg: JPEG image data, JFIF standard 1.01, aspect ratio, density 1x1, segment length 16, baseline, precision 8, 2x4, components 1
4x4.jpg: JPEG image data, JFIF standard 1.01, aspect ratio, density 1x1, segment length 16, baseline, precision 8, 4x4, components 1

@timothyqiu
Copy link
Member

I can reproduce this with JPEGs generated by ImageMagick if I set the colorspace to YCbCr and use a sampling factor of 2x2:

convert -size 1x1 xc:"#fff" -sampling-factor 2x2 -colorspace ycbcr 1x1.jpg

This produces the same backtrace as the MRP.

@akien-mga akien-mga changed the title Godot crashes when importing 1x1 pixel JPEG image. Godot crashes when importing 1x1 pixel JPEG image using YCbCr ans 2x2 sampling factor Oct 9, 2020
@akien-mga akien-mga changed the title Godot crashes when importing 1x1 pixel JPEG image using YCbCr ans 2x2 sampling factor Godot crashes when importing 1x1 pixel JPEG image using YCbCr and 2x2 sampling factor Oct 9, 2020
@alketii
Copy link

alketii commented Nov 1, 2020

This is happening to me also, while trying to import a .dae file with a 1x2 texture.

@akien-mga akien-mga added this to the 4.0 milestone Nov 1, 2020
@Bhu1-V
Copy link
Contributor

Bhu1-V commented Nov 6, 2020

hello,
I want To work on this i understood the problem but i really don't known where to start .
I imported the MRP saw that it got froze at Importing assets dialog and Crashed, so I think i should start by checking code for importing project and importing assets functions but, it's hard to find.

Any help regarding this Is Appreciated.

@akien-mga
Copy link
Member

If you can reproduce the crash, use a debugger to get a stacktrace, which will show you which files are involved.

There's a hint in #42363 (comment):

godot.x11.tools.64.llvm: thirdparty/jpeg-compressor/jpgd.h:278: int jpgd::jpeg_decoder::check_sample_buf_ofs(int) const: Assertion `ofs >= 0' failed.

You can then search which files include this file:

$ rg "jpgd.h"
thirdparty/jpeg-compressor/jpgd.cpp
29:#include "jpgd.h"

thirdparty/jpeg-compressor/jpgd.h
1:// jpgd.h - C++ class for JPEG decompression.

modules/jpg/image_loader_jpegd.cpp
36:#include <jpgd.h>

(But again a stacktrace would show you that the crash in thirdparty/jpeg-compressor/jpgd.h originates in modules/jpg/image_loader_jpegd.cpp, and would additionally give you the relevant line and further call stack for what triggers that image loader call in the editor import code.)

@Bhu1-V
Copy link
Contributor

Bhu1-V commented Nov 6, 2020

So, I checked the code int b = (c_x1 >> 3) * BLOCKS_PER_MCU * 64 + (c_x1 & 7); which is in file thirdparty/jpeg-compressor/jpgd.cpp on Line 2061

above value b becoming negative which eventually on the next line :
int cb10_sample = p_C0Samples[check_sample_buf_ofs(b + y0_base)];
is used to call check_sample_buf_ofs() which checks whether passed argument is positive or not
(check check_sample_buf_ofs() defination ) and closes the program when it's negative -> CAUSE OF ERROR

Then, I tried to read the function in which this whole thing is implemented and found out that it is uint32_t jpeg_decoder::H2V2ConvertFiltered() Fuction.

now i am really stuck on what to do next because i don't see anything wrong in the implementation.

If i am missing something. I'm Happy to learn it.

Bhu1-V added a commit to Bhu1-V/jpeg-compressor that referenced this issue Nov 10, 2020
without this check when an 1 x 1 or 1 x X or X x 1 using sub-sampling. the half_image_size would be -1 which would result in a crash on future steps.
Fixed : godotengine/godot#42363 Caused by this Bug.
GryphonClaw pushed a commit to GryphonClaw/godot that referenced this issue Nov 19, 2020
This make sure that (1x1) , (1 x X) and (X , 1) pixel images using sub-sampling will get correct half_image_size i.e NON-NEGATIVE.
fix : godotengine#42363
akien-mga pushed a commit that referenced this issue Feb 26, 2021
This make sure that (1x1) , (1 x X) and (X , 1) pixel images using sub-sampling will get correct half_image_size i.e NON-NEGATIVE.
fix : #42363

(cherry picked from commit ec6a4c9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment