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

Use of toFile with filename ending in j2c or j2k, without preceeding dot, assumes JPEG2000 output #3672

Closed
bianjunjie1981 opened this issue May 18, 2023 · 15 comments

Comments

@bianjunjie1981
Copy link
Contributor

bianjunjie1981 commented May 18, 2023

JP2 output requires libvips with support for OpenJPEG

sharp.cache(false); // I clear the cache before calling
sharp(path_input)....toFile(path_output)

Now the path_input is fixed, but the picture itself is changed.

If no errors occur, the execution is normal,The output changes with the input,
The cache cleanup is valid.
If an error occurs, the result is cached unless the output path is changed.

for example
JP2 output requires libvips with support for OpenJPEG

path_input the picture itself is changed and is no longer jp2,Errors will occur.
I use a different path_input, error is always going to happen.

Util path_output change.

And the cache isn't just in memory. Restart the process and restart the machine errors still exist.

@lovell
Copy link
Owner

lovell commented May 18, 2023

If path_output ends with a JPEG2000 extension (.jp2, .jpx, .j2k, .j2c) then sharp assumes you want JPG2000 output, which requires libvips compiled with support for OpenJPEG. The prebuilt libvips provided by sharp does not include this, so will generate the "JP2 output requires libvips with support for OpenJPEG" error.

I you want to use one of these file extensions but not use the JPG2000 format for the output then you'll need to tell sharp what format you do want, e.g. for JPEG output try:

- sharp(path_input).toFile(path_output)
+ sharp(path_input).jpeg().toFile(path_output)

@bianjunjie1981
Copy link
Contributor Author

@lovell
Sorry, I don't think my question is very clear.
I'm not talking about converting jp2 errors per se, I'm definitely using .jpeg().

What I'm saying is that when an error is reported, the error is cached
Even if you change the image itself or the image path, as long as the output path is the same,
The error is always cached and returned as execution.

@lovell
Copy link
Owner

lovell commented May 18, 2023

Please provide complete, standalone code that allows someone else to reproduce.

@bianjunjie1981
Copy link
Contributor Author

bianjunjie1981 commented May 18, 2023

@lovell

const sharp = require('sharp');

async function test(){
    try{
        sharp.cache(false);
        await sharp('./test.jp2').resize(300, 300).jpeg().toFile('./output.jpg');
    }catch(e){
        console.error('exception:', e);
    }

    try{
        sharp.cache(false);
        await sharp('./test2.jpg').resize(300, 300).jpeg().toFile('./output.jpg');
    }catch(e){
        console.error('exception:', e);
    }
}

test();

sorry, the picture I raised this error is no longer available
I found a jp2 and tried it. The error was not the same, and it was fine

The previous error was:
Error: JP2 output requires libvips with support for OpenJPEG

Now the error is:
[Error: Input file contains unsupported image format]

Theoretically, I should get two exceptions for executing the file above
Error: JP2 output requires libvips with support for OpenJPEG

@bianjunjie1981
Copy link
Contributor Author

bianjunjie1981 commented May 18, 2023

@lovell

It turned out that the file extension in question was .jpg
But now I'm even going to change this jp2 extension to .jpg

The error is still
[Error: Input file contains unsupported image format]
Instead of
Error: JP2 output requires libvips with support for OpenJPEG

I'm sorry that I didn't keep the pictures.
I found three such pictures among the 170,000 pictures, which caused the above error.
At that time, the repair process,
no matter how to change the original image, set the change path,
as long as that output path is unchanged,
those three executions are consistent and steady error.

@bianjunjie1981
Copy link
Contributor Author

bianjunjie1981 commented May 18, 2023

I did a search for the code
Input file contains unsupported image format
It is reported by c++. It should be pre-detected and not passed.

JP2 output requires libvips with support for OpenJPEG
It should have passed the basic tests and reached execution time

As for why the cache problem occurs
I was not aware of cache(false) and did not clear the error.

If I could reproduce the situation it would look something like this

input1(picture in question)     -> output1 result is exception
input2(normal picture)          -> output1 result is exception
input2(normal picture)          -> output2 result is normal

That is, I feel that because the output path is the same, the exception is cached.

And I restart the process, restart the machine whatever the output path is
The error is still there, so I get the feeling that the cache isn't just in memory.

@bianjunjie1981
Copy link
Contributor Author

@lovell

I figured out a way to export normal images to the output_path where I was having problems.

Steadily reproduces the error, and the error record still exists,
Error: JP2 output requires libvips with support for OpenJPEG

that is If I don't change the output_path, I will never be able to output to it.

@bianjunjie1981
Copy link
Contributor Author

@lovell

I'll post my test code and execution results

var sharp = require('sharp');

async function process(input, output){
    if(!start){ start = +new Date(); }
    try{
        sharp.cache(false);
        await sharp(input).resize(300, 300).jpeg().toFile(output);
        console.log('ok');
    }catch(e){
        console.error('exception:', e);
    }
    cost();
    console.log('-----------------------------------------------------------\n\n');
}

var start = 0;
var cost = function(){
    console.log('cost', +new Date() - start);
    start = +new Date();
}

async function test(){
    // two normal images
    var p1  = './test/1.jpg';
    var p2  = './test/2.jpg';

    // normal output path
    var f1 = './output';
    // the path where error(Error: JP2 output requires libvips with support for OpenJPEG) occurred
    var f2 = '/Users/bianjunjie/Library/Application Support/N.Server/image/cover/jp/om7npcrvpq7pgth9nj2c';


    // if cache cleared, why exception  // Execution time is extremely short

    // different input to same output
    await  process(p1, f1); //ok
    await  process(p2, f1); //ok

    await  process(p1, f2); //exception
    await  process(p2, f2); //exception

    // excute again
    // same input to different output
    await  process(p1, f1); //ok
    await  process(p1, f2); //exception

    await  process(p2, f1); //ok
    await  process(p2, f2); //exception
}

test();
➜  test node sharp.js
ok
cost 19
-----------------------------------------------------------


ok
cost 48
-----------------------------------------------------------


exception: Error: JP2 output requires libvips with support for OpenJPEG
    at errJp2Save (/Users/bianjunjie/Documents/develop/test/node_modules/sharp/lib/output.js:34:26)
    at Sharp.toFile (/Users/bianjunjie/Documents/develop/test/node_modules/sharp/lib/output.js:79:11)
    at process (/Users/bianjunjie/Documents/develop/test/sharp.js:7:52)
    at test (/Users/bianjunjie/Documents/develop/test/sharp.js:39:12)
cost 4
-----------------------------------------------------------


exception: Error: JP2 output requires libvips with support for OpenJPEG
    at errJp2Save (/Users/bianjunjie/Documents/develop/test/node_modules/sharp/lib/output.js:34:26)
    at Sharp.toFile (/Users/bianjunjie/Documents/develop/test/node_modules/sharp/lib/output.js:79:11)
    at process (/Users/bianjunjie/Documents/develop/test/sharp.js:7:52)
    at test (/Users/bianjunjie/Documents/develop/test/sharp.js:40:12)
cost 0
-----------------------------------------------------------


ok
cost 10
-----------------------------------------------------------


exception: Error: JP2 output requires libvips with support for OpenJPEG
    at errJp2Save (/Users/bianjunjie/Documents/develop/test/node_modules/sharp/lib/output.js:34:26)
    at Sharp.toFile (/Users/bianjunjie/Documents/develop/test/node_modules/sharp/lib/output.js:79:11)
    at process (/Users/bianjunjie/Documents/develop/test/sharp.js:7:52)
    at test (/Users/bianjunjie/Documents/develop/test/sharp.js:45:12)
cost 1
-----------------------------------------------------------


ok
cost 47
-----------------------------------------------------------


exception: Error: JP2 output requires libvips with support for OpenJPEG
    at errJp2Save (/Users/bianjunjie/Documents/develop/test/node_modules/sharp/lib/output.js:34:26)
    at Sharp.toFile (/Users/bianjunjie/Documents/develop/test/node_modules/sharp/lib/output.js:79:11)
    at process (/Users/bianjunjie/Documents/develop/test/sharp.js:7:52)
    at test (/Users/bianjunjie/Documents/develop/test/sharp.js:48:12)
cost 1
-----------------------------------------------------------

@lovell
Copy link
Owner

lovell commented May 18, 2023

There's a lot going on here. Perhaps you could create a standalone repo with code and images that allows someone else to reproduce. Please try to make it as minimal as possible.

@bianjunjie1981
Copy link
Contributor Author

@lovell
I'm sorry I don't have those images anymore
The exception has to be raised first
And then there's the reproducible

My main question now is where is the cache
How can I clear it

@bianjunjie1981
Copy link
Contributor Author

bianjunjie1981 commented May 19, 2023

@lovell

I can now replicate the mistake at will and figure out what the problem is.
If the input source is not a jp2 image, but the output path matches the RE then it repeats stably.

const jp2Regex = /\.jp[2x]|j2[kc]$/i;

The RE is wrong, is too loose and the match is too long
/Users/bianjunjie/Library/Application Support/N.Server/image/cover/jp/om7npcrvpq7pgth9nj2c
It fits perfectly with this regular

Problems occurred with the three paths because the directory section had "."
The ending matches so it's considered a jp2 file

3hoyz5dcx8efl4dypgnj2k
a4ygmscovuepz8crye6j2c
jpom7npcrvpq7pgth9nj2c

sharp/lib/output.js

line 32
const jp2Regex = /\.jp[2x]|j2[kc]$/i;

function toFile (fileOut, callback) {
let err;
if (!is.string(fileOut)) {
err = new Error('Missing output file path');
} else if (is.string(this.options.input.file) && path.resolve(this.options.input.file) === path.resolve(fileOut)) {
err = new Error('Cannot use same file for input and output');

// line 78
} else if (jp2Regex.test(fileOut) && !this.constructor.format.jp2k.output.file) {

@lovell
Copy link
Owner

lovell commented May 20, 2023

Well done for tracking this down, thank you. Happy to accept a PR to fix this if you're able.

@lovell lovell added bug and removed question labels May 20, 2023
@lovell lovell changed the title toFile, result cache Use of toFile with filename ending in j2c or j2k, without preceeding dot, assumes JPEG2000 output May 20, 2023
@bianjunjie1981
Copy link
Contributor Author

bianjunjie1981 commented May 20, 2023

@lovell

Although RE is less rigorous, it can still be used
However, if the test uses RE for the entire path and is rigorous, I'm afraid the expression will be too long
Instead, fetch the extension and then test the extension
This file already has require('path')

I suggest
``
sharp/lib/output.js
line 78

jp2Regex.test(fileOut)
->
jp2Regex.test(path.extname(fileOut))

@bianjunjie1981
Copy link
Contributor Author

@lovell
I created a pull requrest, Please review.

#3674

@lovell lovell added this to the v0.32.2 milestone Jun 5, 2023
@lovell
Copy link
Owner

lovell commented Jul 11, 2023

v0.32.2 now available, thank you for the PR.

@lovell lovell closed this as completed Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants