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

Add other pnm (pbm, pgm, pam) decoders #695

Merged
merged 23 commits into from Nov 30, 2017

Conversation

Projects
None yet
3 participants
@HeroicKatora
Contributor

HeroicKatora commented Nov 17, 2017

Adds decoders and format recognition for the following formats from the netbpm package:
Portable Bit Map, Portable Gray Map and Portable Arbitrary Map

Details

The pam format decoder recognizes the tuple types BLACKANDWHITE, GRAYSCALE and RGB and explicitely recognizes but rejects their _ALPHA variants (support for these might come in another patch).

Tests for the new formats are not yet included but will arrive in the next week, this should stay open until then.

This should deprecate the pam module, not yet part of this pull request. In case it is desired to explicitely test for the ppm format instead, the decoder class offers a method to query the specific image subtype after the header has been read and a wrapper for ppm specificially has been put into the place of the old decoder.

Related Issues

This closes #664

@bvssvni

This comment has been minimized.

Member

bvssvni commented Nov 23, 2017

Tell me when you're ready.

@HeroicKatora

This comment has been minimized.

Contributor

HeroicKatora commented Nov 23, 2017

I'll add some more tests tomorrow and on the weekend, see if anything turns up in pam especially. Then this should be good to go probably on Saturday. One question, should mod pnm be deprecated right away or not? And if so, what's the preferred way?

@bvssvni

This comment has been minimized.

Member

bvssvni commented Nov 23, 2017

@nwin what do you think?

HeroicKatora added some commits Nov 25, 2017

@HeroicKatora

This comment has been minimized.

Contributor

HeroicKatora commented Nov 26, 2017

Tests are currently done in pnm as far as pam, pbm and pgm are concerned and in ppm for ppm. Turned out I still had bugs before. All good on my end, ready to go.

@nwin

This comment has been minimized.

Contributor

nwin commented Nov 26, 2017

I think it would be best to deprecate mod pnm right away.

@HeroicKatora

This comment has been minimized.

Contributor

HeroicKatora commented Nov 26, 2017

This brings up something interesting, maybe one would want to assert the image format to be ppm specifically. There is currently no replacement for it when ImageFormat::PPM is removed. Maybe the ImageFormat::PNM could get the additional subtype parameter from the decoder struct so this can be queried/specified.

@HeroicKatora

This comment has been minimized.

Contributor

HeroicKatora commented Nov 26, 2017

Also, the encoder is still only written for ppm, so I've just deprecated the decoder for the moment.

@bvssvni

This comment has been minimized.

Member

bvssvni commented Nov 28, 2017

I think we can merge this now and make changes to ImageFormat later if needed.

@HeroicKatora

This comment has been minimized.

Contributor

HeroicKatora commented Nov 28, 2017

Possibly when the encoder has been transported to pnm or at 0.18. Who knows, maybe both at once? But the former would be a good reason, as it would support specifying the intended output encoding in a uniform way with one implementation.

@bvssvni

This comment has been minimized.

Member

bvssvni commented Nov 30, 2017

Merging.

@bvssvni bvssvni merged commit c12a493 into PistonDevelopers:master Nov 30, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment