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

pnm-plugin #4253

Merged
merged 12 commits into from
May 4, 2024
Merged
52 changes: 34 additions & 18 deletions src/doc/builtinplugins.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1678,22 +1678,23 @@ and :ref:`sec-imageinput-ioproxy`) as well as the `set_ioproxy()` methods.
PNM / Netpbm
===============================================

The Netpbm project, a.k.a. PNM (portable "any" map) defines PBM, PGM,
and PPM (portable bitmap, portable graymap, portable pixmap) files.
The Netpbm project, a.k.a. PNM (portable "any" map) defines PBM, PGM, PPM
and later added PFM (portable float map) as a set of simple image formats
(portable bitmap, portable graymap, portable pixmap) files.
Without loss of generality, we will refer to these all collectively as
"PNM." These files have extensions :file:`.pbm`, :file:`.pgm`, and
:file:`.ppm` and customarily correspond to bi-level bitmaps, 1-channel
grayscale, and 3-channel RGB files, respectively, or :file:`.pnm` for
those who reject the nonsense about naming the files depending on the
"PNM." These files have extensions :file:`.pbm`, :file:`.pgm`,
:file:`.ppm`, :file:`.pfm` and customarily correspond to bi-level bitmaps,
1-channel grayscale, and 3-channel RGB files, respectively, or :file:`.pnm`
for those who reject the nonsense about naming the files depending on the
number of channels and bitdepth.

PNM files are not much good for anything, but because of their
historical significance and extreme simplicity (that causes many
"amateur" programs to write images in these formats), OpenImageIO
supports them. PNM files do not support floating point images, anything
other than 1 or 3 channels, no tiles, no multi-image, no MIPmapping.
It's not a smart choice unless you are sending your images back to the
1980's via a time machine.
PNM files widely used in the Unix world as simple ASCII or binary image
lgritz marked this conversation as resolved.
Show resolved Hide resolved
files that are easy to read and write. They are not compressed, and are
not particularly efficient for large images. They are not widely used in
the professional graphics world, but because of their historical
significance and extreme simplicity, OpenImageIO supports them.
PNM files do not support anything other than 1 or 3 channels, no tiles,
no multi-image, no MIPmapping.

ssh4net marked this conversation as resolved.
Show resolved Hide resolved
**Attributes**

Expand All @@ -1708,11 +1709,6 @@ It's not a smart choice unless you are sending your images back to the
- int
- The true bits per sample of the file (1 for true PBM files, even
though OIIO will report the ``format`` as UINT8).
* - ``pnm:binary``
- int
- nonzero if the file itself used the PNM binary format, 0 if it used
ASCII. The PNM writer honors this attribute in the ImageSpec to
determine whether to write an ASCII or binary file.
ssh4net marked this conversation as resolved.
Show resolved Hide resolved

**Configuration settings for PNM input**

Expand All @@ -1731,6 +1727,13 @@ attributes are supported:
- ptr
- Pointer to a ``Filesystem::IOProxy`` that will handle the I/O, for
example by reading from memory rather than the file system.
* - ``pnm:bigendian``
- int
- If nonzero, the PNM file is big-endian (the default is little-endian).
* - ``pnm:pfmflip``
- int
- If nonzero, the PFM (float) file is flipped upside-down (the default
is flipped).
ssh4net marked this conversation as resolved.
Show resolved Hide resolved
ssh4net marked this conversation as resolved.
Show resolved Hide resolved

**Configuration settings for PNM output**

Expand All @@ -1753,6 +1756,19 @@ control aspects of the writing itself:
- ptr
- Pointer to a ``Filesystem::IOProxy`` that will handle the I/O, for
example by writing to a memory buffer.
* - ``pnm:bigendian``
- int
- If nonzero, the PNM file is big-endian (the default is little-endian).
* - ``pnm:binary``
- int
- nonzero if the file itself used the PNM binary format, 0 if it used
ASCII. The PNM writer honors this attribute in the ImageSpec to
determine whether to write an ASCII or binary file.
Float PFM files are always written in binary format.
* - ``pnm:pfmflip``
- int
- If nonzero, the PFM (float) file is flipped upside-down (the default
is flipped).
ssh4net marked this conversation as resolved.
Show resolved Hide resolved

**Custom I/O Overrides**

Expand Down
23 changes: 19 additions & 4 deletions src/pnm.imageio/pnminput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0
// https://github.com/AcademySoftwareFoundation/OpenImageIO

#include <cstdio>
#include <cstdlib>
#include <fstream>
#include <string>
Expand All @@ -12,6 +13,8 @@

OIIO_PLUGIN_NAMESPACE_BEGIN

#define DBG if (0)

//
// Documentation on the PNM formats can be found at:
// http://netpbm.sourceforge.net/doc/pbm.html (B&W)
Expand Down Expand Up @@ -188,6 +191,7 @@ unpack_floats(const unsigned char* read, float* write, imagesize_t numsamples,
bool
PNMInput::read_file_scanline(void* data, int y)
{
DBG std::cerr << "PNMInput::read_file_scanline(" << y << ")\n";
if (y < m_y_next) {
// If being asked to backtrack to an earlier scanline, reset all the
// way to the beginning, right after the header.
Expand All @@ -202,9 +206,11 @@ PNMInput::read_file_scanline(void* data, int y)
for (; good && m_y_next <= y; ++m_y_next) {
// PFM files are bottom-to-top, so we need to seek to the right spot
if (m_pnm_type == PF || m_pnm_type == Pf) {
int file_scanline = m_spec.height - 1 - (y - m_spec.y);
auto offset = file_scanline * m_spec.scanline_bytes();
m_remaining = m_after_header.substr(offset);
if (m_spec.get_int_attribute("pnm:pfmflip", 1) == 1) {
int file_scanline = m_spec.height - 1 - (y - m_spec.y);
auto offset = file_scanline * m_spec.scanline_bytes();
m_remaining = m_after_header.substr(offset);
}
}

if ((m_pnm_type >= P4 && m_pnm_type <= P6) || m_pnm_type == PF
Expand All @@ -219,6 +225,7 @@ PNMInput::read_file_scanline(void* data, int y)
if (size_t(numbytes) > m_remaining.size())
return false;
buf.assign(m_remaining.begin(), m_remaining.begin() + numbytes);

m_remaining.remove_prefix(numbytes);
}

Expand Down Expand Up @@ -313,6 +320,7 @@ PNMInput::read_file_header()
int bps = int(ceilf(logf(m_max_val + 1) / logf(2)));
if (bps < 8)
m_spec.attribute("oiio:BitsPerSample", bps);
m_spec.attribute("pnm:pfmflip", 0);
} else {
//Read scaling factor
if (!nextVal(m_scaling_factor))
Expand All @@ -327,6 +335,7 @@ PNMInput::read_file_header()
m_spec = ImageSpec(width, height, m_pnm_type == PF ? 3 : 1,
TypeDesc::FLOAT);
m_spec.attribute("pnm:bigendian", m_scaling_factor < 0 ? 0 : 1);
m_spec.attribute("pnm:binary", 1);
}
m_spec.attribute("oiio:ColorSpace", "Rec709");
return true;
Expand All @@ -339,7 +348,13 @@ PNMInput::open(const std::string& name, ImageSpec& newspec,
const ImageSpec& config)
{
ioproxy_retrieve_from_config(config);
return open(name, newspec);
if (!open(name, newspec)) {
errorfmt("Could not parse spec for file \"%s\"", name);
return false;
}

m_spec["pnm:pfmflip"] = config.get_int_attribute("pnm:pfmflip", 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

m_spec is for communicating what's in the file to the caller.

config is for the caller to give us instructions about how to read.

So it's unusual to copy something from the config into m_spec. I suspect you are merely trying to communicate that config hint to the later reading functions, but usually what we do in a case like this is just have a bool as a private member of the PNMInput, set it here, and use it as needed in the read_scanline function.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think at the very least, we should only set this in m_spec if it really is a pfm file, not for the non-float varieties.

return true;
}


Expand Down