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

Format hint in image_info->magick doesn't work when loading from file #6242

Closed
kleisauke opened this issue Apr 8, 2023 · 6 comments
Closed

Comments

@kleisauke
Copy link
Contributor

ImageMagick version

7.1.1-6

Operating system

Linux

Operating system, version and so on

Fedora 37

Description

Hinting the format in image_info->magick doesn't work when loading from a file. For example, when loading a NEF image and initializing image_info->magick to "NEF" the image appears to load via the TIFF loader (see reproducer below).

Reverting commit 6f7cebf seems to fix this.

--- a/MagickCore/image.c
+++ b/MagickCore/image.c
@@ -2855,8 +2855,7 @@ MagickExport MagickBooleanType SetImageInfo(ImageInfo *image_info,
         }
     }
   *component='\0';
-  if (*image_info->magick == '\0')
-    GetPathComponent(image_info->filename,ExtensionPath,component);
+  GetPathComponent(image_info->filename,ExtensionPath,component);
   if (*component != '\0')
     {
       /*

(This corresponds to commit ImageMagick/ImageMagick6@2cf7bfe in ImageMagick 6.x)

Steps to Reproduce

/* Compile with:
 * gcc -g -Wall test-nef.c `pkg-config MagickCore --cflags --libs` -o test-nef
 *
 * Run with:
 * ./test-nef _DSC4175.NEF
 *
 * Expected output: 4284x2844
 * Actual output: 160x120
 */
#include <stdio.h>
#include <assert.h>

#include <MagickCore/MagickCore.h>

int
main(int argc, char **argv)
{
	ExceptionInfo
		*exception;

	Image
		*image;

	ImageInfo
		*image_info;

	if (argc != 2) {
		printf("Usage: %s image\n", argv[0]);
		return 1;
	}

	MagickCoreGenesis(*argv, MagickTrue);
	image_info = AcquireImageInfo();
	(void) CopyMagickString(image_info->filename, argv[1], MaxTextExtent);
	(void) CopyMagickString(image_info->magick, "NEF", MaxTextExtent);
	exception = AcquireExceptionInfo();
	image = ReadImage(image_info, exception);

	assert(image);
	printf("%ldx%ld\n", image->columns, image->rows);

	image = DestroyImage(image);
	exception = DestroyExceptionInfo(exception);
	image_info = DestroyImageInfo(image_info);
	MagickCoreTerminus();

	return 0;
}

Images

_DSC4175.zip

@kleisauke
Copy link
Contributor Author

Thanks! I'm not sure if this is on purpose, but I noticed that image_info->filename has a higher priority than image_info->magick, i.e. on the above reproducer:

$ cp _DSC4175.NEF x.tiff
$ ./test-nef x.tiff
160x120
$ ./test-nef _DSC4175.NEF
4284x2844

I would expect both commands to return 4284x2844.

@kleisauke
Copy link
Contributor Author

Ah, it looks like image_info->magick is still being ignored somehow.

$ cp _DSC4175.NEF x
$ ./test-nef x
160x120

@urban-warrior
Copy link
Member

The type of an image can be implied by its file extension, such as NEF in this case. However, the internal "magic" numbers of the image can override this implicit format. In the case where an image has a TIFF signature, it will be read by the TIFF coder instead. If you want to explicitly set the image type, you can do so by prefixing the image filename. This explicit file type will take precedence over any magic number or implicit file extension.

It's possible that your unit test used to work in the past, but we likely discovered and fixed a bug that caused a side effect, leading to your unit test failure. To achieve the desired outcome, it's best to use an explicit filename.

$ ./test-nef nef:_DSC4175.NEF
4284x2844

@kleisauke
Copy link
Contributor Author

I see, but NEF images are registered as ExplicitFormatType.

entry->format_type=ExplicitFormatType;

So, I would assume that setting image_info->magick to "NEF" would not determine the image format from the first few bytes of the file, thus preventing it from being loaded by the TIFF coder.

How about this patch?:

--- a/magick/image.c
+++ b/magick/image.c
@@ -2914,6 +2914,9 @@ MagickExport MagickBooleanType SetImageInfo(ImageInfo *image_info,
     {
       (void) CopyMagickString(magic,image_info->magick,MaxTextExtent);
       magick_info=GetMagickInfo(magic,sans_exception);
+      if ((magick_info != (const MagickInfo *) NULL) &&
+          (magick_info->format_type == ExplicitFormatType))
+          image_info->affirm=MagickTrue;
       if (frames == 0)
         GetPathComponent(image_info->filename,CanonicalPath,filename);
       else

This would fix all the test cases mentioned earlier.

@urban-warrior
Copy link
Member

We'll apply the patch. We'll need to run all our units tests (multi-thousands) before we can release the patch. If we run into any issues, we'll let you know.

@kleisauke
Copy link
Contributor Author

Thanks! I'll close, feel free to re-open if this causes any issues.

For reference, this fix was released in version 7.1.1-7 and 6.9.12-85.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Apr 23, 2023
7.1.1-8 -  2023-04-22

Fixed

    Added checks to make sure all bytes were read to resolve #6267. #6267

Commits

    beta release 35ec38f
    Treat warnings as errors in the MacOS build and enabled more warnings for that build. 02b2aa6
    Corrected compare. 35505ca
    Not longer export methods that are not used in other parts of the library. 01251e5
    No longer call ParseMetaGeometry twice when we don't add a thumbnail. 6a94dd8
    Fix typo that caused a division by zero in #6263. 78347b5
    don't reduct 3 to 1 channel gray if meta channels are present a8f6186
    The depth must be specified when reading a map image. 2d6e2e9
    validate pixel offset 90e067c
    validate pixel offset d92cb0e
    release 920f792

7.1.1-7 - 2023-04-16

Merged

    PixelChannel: Add invalid channel field #6230

Commits

    beta release 3bd5ce1
    account for extra samples 9a9896f
    The quantum extent should also including the pad. b019133
    Another improvement of calculating the size of the extent. 3151fd8
    The padding is per pixel. dc0556c
    Cosmetic. ad36935
    Use the new API when available. 8b12fc1
    don't cut off letters (ImageMagick/ImageMagick#6221) fcf2674
    Moved static declaration. 8eaadcf
    Removed unused lt_dlexit define. d454d11
    Removed unused NTSetSearchPath method. 43ea02a
    Windows doesn't need a call to lt_dlinit. 48e9207
    There is no need to call WSAStartup because we will always have version 2.2 on the supported platforms. 24990f7
    Moved linking of lib inside other check. b13fabd
    More cleanup of header files. d048972
    Silence warnings for when the distribute-cache is not supported. 13841a1
    support --enable-dpc configure option fc7bb1d
    Fixed build error. 178f677
    theoretically a more intuitive brighness contrast algorithm (ImageMagick/ImageMagick#6079) cdc73fb
    revert format hint (ImageMagick/ImageMagick#6242) 9e1492e
    Use the new MAGICKCORE_DPC_SUPPORT flag. 7c8a486
    Removed define that always had the same value. 8e5fbea
    We only need to link ws2_32.lib on Windows when we have enabled DPC support. 1c4f052
    identify correct format ef12f38
    Restored the call to WSAStartup because we do need to do this when we use winsock2. 1bc9391
    Correct define check. 610a8a5
    Corrected checks that determine if we actually are able to support dpc. 70cd76b
    revert 723b63a
    correct slope/intercept 405c61a
    correct intercept b29bcd9
    eliminate compiler warnings (ImageMagick/ImageMagick#6230) fba0e38
    cosmetic 1fe2162
    eliminate compiler warning 3b84c79
    release e4b3733
    beta release 3b5b4a1
    if the image type is explicit, use the file extension if possible (ImageMagick/ImageMagick#6242) 67aa0ed
    Removed unused return value. 0d5e3a0
    Use version number instead. 1bc3bdc
    Silence warning by casting to size_t instead where negative values will overflow in a large value. 8def9df
    Silenced warnings. 7285688
    Silenced warnings. 864e6db
    Added missing break. 58b6568
    Moved devcontainer into security folder. 386f3b8
    Reverted patch and silenced warning. 4602557
    Added name to the container. f9b5142
    Added missing break. 9754705
    Silenced MagickCore warnings. 427e443
    Silenced coders warnings. 8f78bf4
    Corrected devcontainer name. ecc72e5
    Unreference in else statement instead. 69da709
    Removed unused argument. 890b4c7
    Removed unused return values. 1309276
    Only define GetICCProperty when build with LCMS delegate. bacfcad
    Silenced warning. a489b2a
    Silenced warnings in MagickWand. bf1e065
    Added devcontainer for local development. 18fd137
    Use define because the fallthrough attribute is only supported by gcc. 312aaf9
    Added Dockerfile to the editorconfig. 38ecac1
    Use different notation to fix the macos build. 35152a1
    Added devcontainer that uses the clang compiler. fc6f751
    Changed build to treat all warnings as errors and check for more warnings. 2a003f8
    Silenced warning. f85c832
    Change code to make it compile with -Wall -Wextra -Werror b164646
    Silenced warning. cdba683
    Ignore unused-function and builtin-declaration-mismatch warnings to work around autoconf issues. f311596
    Clang needs different flags to fix the build. 3e49a05
    Also build with clang in the daily build. 717d8d7
    Include compiler in the title. 50b4062
    Silence warnings that occur when freetype is enabled. 3f9cfbd
    Silenced more warnings. 7c809d7
    Silence the warning differently. feaa675
    elimiate compiler warnings f49eeca
    eliminate compiler warnings c40db4e
    eliminate compiler warnings 9d85754
    eliminate compiler warning 7157e1a
    expand channel map by 1 cc97c3a
    initialize max channels + 1 bdd4599
    add additional checks for casting double to size_t f7b5682
    cosmetic 77f6713
    identify z component of chromaticity fecfed4
    Refactor code to make it more readable. 9783994
    Corrected compare. c13cada
    Also skip writing the exif/tiff resolution properties when the pHYs chunk is written. d4f233b
    improved range checking 4daec2d
    cosmetic. 8066117
    Removed unused return value. d3cf508
    consistent method to check for alpha channel 242e940
    Correct comment. 43aa790
    Added method to update the density and orientation in the xmp profile. fc4f67b
    Corrected value for tiff:ResolutionUnit. c9f17dc
    Cosmetic. 589856f
    Removed debug printf statement... fecd253
    round crop width properly adda986
    Install more dependencies for the macOS build. 4a52f67
    Silence warning. 30f3676
    release 21d049b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants