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

Poor image quality #172

Closed
rawbertp opened this issue Nov 1, 2019 · 17 comments
Closed

Poor image quality #172

rawbertp opened this issue Nov 1, 2019 · 17 comments

Comments

@rawbertp
Copy link
Contributor

rawbertp commented Nov 1, 2019

Is your feature request related to a problem? Please describe.
The jpgs in the data folder only are around 600K in size (whereas they should be ~3M). I haven't found any setting that would allow me to set the compression but it appears that during image processing the JPGs are compressed/whatever.

Describe the solution you'd like
Configuration options exist that allow the user to control the image processing behavior, e.g. turn off any compression/allow to keep and use the original images.

Describe alternatives you've considered
n/a

Additional context

pi@raspberrypi:/var/www/html/data/images $ ls -ltrh
total 18M
...
-rw-r--r-- 1 www-data www-data 621K Nov  1 10:08 8136336c55f8ce67d7a679540454a2a8.jpg
@andi34
Copy link
Collaborator

andi34 commented Nov 1, 2019

Please edit your opening post using the bug or feature template, will be reopened once the opening post is updated.

@andi34 andi34 closed this as completed Nov 1, 2019
@andi34 andi34 added invalid and removed invalid labels Nov 1, 2019
@andi34
Copy link
Collaborator

andi34 commented Nov 1, 2019

data/images or data/tmp ?

@andi34 andi34 reopened this Nov 1, 2019
@rawbertp
Copy link
Contributor Author

rawbertp commented Nov 1, 2019

It's about data/images (I updated the original post accordingly). It appears the original JPGs are in the data/tmp folder. However, to me a tmp folder is something rather short-lived?!

@andi34
Copy link
Collaborator

andi34 commented Nov 1, 2019

tmp folder was introduced to store the original before a filter is applied.

@rawbertp
Copy link
Contributor Author

rawbertp commented Nov 1, 2019

Alright, thanks for that. So maybe it could be renamed to original or something like that which would be far less confusing. Only a cosmetic issue, though.

However, as I have not enabled/used any filters, why are the images in images 1/5 the size of the originals? And even when applying filters it would be nice to have an option to set the compression level/quality loss. :)

@andi34
Copy link
Collaborator

andi34 commented Nov 1, 2019

You can adjust the quality here if needed:
https://github.com/andreknieriem/photobooth/blob/master/api/applyEffects.php#L89

Change from 80 to 100.

@andi34
Copy link
Collaborator

andi34 commented Nov 1, 2019

Alright, thanks for that. So maybe it could be renamed to original or something like that which would be far less confusing. Only a cosmetic issue, though.

You can adjust the folder name via config/admin panel already.

@rawbertp
Copy link
Contributor Author

rawbertp commented Nov 1, 2019

You can adjust the quality here if needed:
https://github.com/andreknieriem/photobooth/blob/master/api/applyEffects.php#L89

Change from 80 to 100.

Thanks for this hint, will do!

You can adjust the folder name via config/admin panel already.

Yeah, you're right! I haven't considered that as tmp to me is not a directory for persistent files. Now I know better...

@rawbertp
Copy link
Contributor Author

rawbertp commented Nov 1, 2019

Interestingly, when changing imagejpeg($imageResource, $filename_photo, 80); to imagejpeg($imageResource, $filename_photo, 100); the JPG in images is now twice the size of the original in tmp. How does that make sense again?

www-data@raspberrypi:~/html/data$ find . -name 4f538fab4387de592472991c0c0eb189.jpg -exec ls -lh {} \;
-rw-r--r-- 1 www-data www-data 3.0M Nov  1 10:57 ./tmp/4f538fab4387de592472991c0c0eb189.jpg
-rw-r--r-- 1 www-data www-data 73K Nov  1 10:57 ./keying/4f538fab4387de592472991c0c0eb189.jpg
-rw-r--r-- 1 www-data www-data 18K Nov  1 10:57 ./thumbs/4f538fab4387de592472991c0c0eb189.jpg
-rw-r--r-- 1 www-data www-data 6.4M Nov  1 10:57 ./images/4f538fab4387de592472991c0c0eb189.jpg

@andi34
Copy link
Collaborator

andi34 commented Nov 1, 2019

GD is used for image processing here. it is creating a new image from the original file which is used by Photobooth. it's not a real 1:1 copy like copy and paste.

@andi34
Copy link
Collaborator

andi34 commented Nov 1, 2019

You might want to take a look here too:
#117

@sualko
Copy link
Collaborator

sualko commented Nov 1, 2019

You can adjust the folder name via config/admin panel already.

That doesn't solve the issue and is dangerous. Assume someone implements a garbage collector for the temp folder, than all your images are gone.

@andi34
Copy link
Collaborator

andi34 commented Nov 1, 2019

You can adjust the folder name via config/admin panel already.

That doesn't solve the issue and is dangerous. Assume someone implements a garbage collector for the temp folder, than all your images are gone.

If it's dangerous why we allow to change the path via config / admin panel?

@rawbertp
Copy link
Contributor Author

rawbertp commented Nov 1, 2019

The question is, what is the main purpose of this folder? From what I've seen so far in the code, it doesn't really serve as a temporary folder as such. It's rather a permanent storage for the original files and hence should be probably renamed to reflect its actual purpose.

@andi34
Copy link
Collaborator

andi34 commented Nov 3, 2019

Maybe we can think about renaming it to original in the next major version bump @sualko ?

api/applyEffects.php:$filename_tmp = $config['foldersAbs']['tmp'] . DIRECTORY_SEPARATOR . $file;
api/applyEffects.php:    $collageBasename = substr($filename_tmp, 0, -4);
api/applyEffects.php:    if (!createCollage($collageSrcImagePaths, $filename_tmp)) {
api/applyEffects.php:if (!file_exists($filename_tmp)) {
api/applyEffects.php:$imageResource = imagecreatefromjpeg($filename_tmp);
api/takePic.php:$filename_tmp = $config['foldersAbs']['tmp'] . DIRECTORY_SEPARATOR . $file;
api/takePic.php:    takePicture($filename_tmp);
api/takePic.php:    $basename = substr($filename_tmp, 0, -4);
config/config.inc.php:$config['folders']['tmp'] = 'data/tmp';
lib/configsetup.inc.php:                'tmp' => [
lib/configsetup.inc.php:                        'placeholder' => 'tmp',
lib/configsetup.inc.php:                        'name' => 'folders[tmp]',
lib/configsetup.inc.php:                        'value' => $config['folders']['tmp']
resources/js/core.js:        const tempImageUrl = config.folders.tmp + '/' + result.file;
resources/lang/de.js:    'folders_tmp': 'tmp Ordner',
resources/lang/en.js:    'folders_tmp': 'tmp Folder',
resources/lang/es.js:    'folders_tmp': 'Carpeta búfer',
resources/lang/fr.js:    'folders_tmp': 'Dossier tmp',
resources/lang/gr.js:    'folders_tmp': 'tmp φάκελο',

Not much lines to adjust.

And yeah, somehow it is a temporary folder as it is used for the original files before they get adjusted for the booth. No user can see them on the webpage.

We could also think about adding an option to keep or remove the original files.

@sualko
Copy link
Collaborator

sualko commented Nov 5, 2019

Maybe we can think about renaming it to original in the next major version bump @sualko ?

That would be an option, but the name should always reflect the use of it. I think we can also skip the jpeg conversion, if no filters/borders and so on are applied. That would also enhance the performance.

@sualko
Copy link
Collaborator

sualko commented Nov 5, 2019

#175 was merged, so I close this issue.

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

3 participants