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

Save image with specified format #988

Merged
merged 4 commits into from Jul 13, 2019
Merged

Conversation

Bruflot
Copy link
Member

@Bruflot Bruflot commented Jul 11, 2019

Ran into the use case where I wanted to save an image without specifying the file extension in the filename. This PR lets you do just that:

img.save_with_format(&"path/to/img_file", ImageFormat::PNG);

I'm not sure if this is a wanted addition or not, so I'm mostly looking for feedback.

I used the image::ImageFormat enum for accepting formats. This enum doesn't include pbm, pgm, ppm, and pam, so I left those out as I'm not familiar enough with the library to make changes to the enum. Alternatively we can just make another enum.


I license past and future contributions under the dual MIT/Apache-2.0 license,
allowing licensees to chose either at their option.

Copy link
Member

@HeroicKatora HeroicKatora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code itself looks very neat overall.

src/buffer.rs Show resolved Hide resolved
src/dynimage.rs Show resolved Hide resolved
@HeroicKatora
Copy link
Member

HeroicKatora commented Jul 11, 2019

This enum doesn't include pbm, pgm, ppm, and pam, so I left those out as I'm not familiar enough with the library to make changes to the enum.

The PNM variant uses pam for all encoders, since the format is flexible enough to be basically a superset of the other netbpm formats. Nothing to worry about. It's not the most used format (without having any stats for it but the encoder didn't exist for 5 years 😉 ) and can be added afterwards if required.

@HeroicKatora
Copy link
Member

I'm not sure if this is a wanted addition or not, so I'm mostly looking for feedback.

Just to be clear. YES! Definitely very much appreciated. This is also the first and a necessary step towards #982 . And it addresses #834 partially. Just making a note here, so that we don't forget to mark it.

@Bruflot
Copy link
Member Author

Bruflot commented Jul 11, 2019

Thanks for the feedback! 😄 I made the changes to the docs, and exposed the save_buffer_with_format function in lib.rs (I forgot to do that earlier).

@HeroicKatora HeroicKatora merged commit 7c5873c into image-rs:master Jul 13, 2019
@HeroicKatora
Copy link
Member

Great work, thank you. Want to join image-rs? You can be invited to the image-rs collaborators team, which allows you access to this and the other repos if you want. See the contribution policy and code of conduct.

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.

None yet

2 participants