What is wrong with my code to write netbpm images?

Question

I am currently working on a project that needs to output generated images. I have decided to the use greyscale netbpm format due to its simplicity. One of these images is a heightmap. My code to export it is as follows:

#include <array>
#include <fstream>
#include <iostream>

int main(){
    constexpr unsigned LENGTH = 4;
    std::array<double, LENGTH*LENGTH> m_data = {
        0, 0, 0, 0,  
        0, 1, 1, 0,
        0, 1, 1, 0,
        0, 0, 0, 0
    };

    std::ofstream fout;
    fout.exceptions(std::ios::badbit | std::ios::failbit);
    fout.open("test.pgm", std::ios::binary);
    fout << "P5 " << LENGTH << " " << LENGTH << "\n256\n";

    for(unsigned i=0;i<LENGTH*LENGTH;++i){
        unsigned char brightness = m_data[i]*255;
        std::cout << m_data[i] << std::endl;
        fout.write(reinterpret_cast<char*>(&brightness), sizeof(unsigned char));
    }

    return 0;
}

This code looks correct to me, after reading the specification for the netbpm format on Wikipedia. Yet, when I attempt to open this image in either Gimp or Clion there are various errors:

The file is saved with a pgm extension. What is wrong with my code?

Could you perhaps post a [mcve] or [SSCCE](http://www.sscce.org). — Jesper Juhl, Jan 02 '19 at 20:31
`unsigned char brightness = m_data[i]*256;` doesn't look right. What are you trying to do with that? — NathanOliver, Jan 02 '19 at 20:31
@NathanOliver m_data is an std::array of doubles in the range [0, 1], and is a member object of HeightMap. I am converting each double to the range [0, 255] and storing it an unsigned char, as the internal binary format of unsigned char matches that of the netbpm format. Each unsigned char or entry in m_data contains exactly one pixel. — john01dav, Jan 02 '19 at 20:33
@JesperJuhl I can, but there is a lot of supporting code needed to make it work (ie. something to generate an image to save). Is there something more specific that you are looking for that I can provide instead? — john01dav, Jan 02 '19 at 20:34
Where are you writing the max value (which should come after the image sizes according to the wikipedia page you linked)? — UnholySheep, Jan 02 '19 at 20:34
You can reproduce the problem with expository input and provide us with that, as the article Jesper linked to explains. You ought to have done this in your debugging phase! — Lightness Races in Orbit, Jan 02 '19 at 20:34
@ravenspoint I'm not sure what you mean by that, but changing 256 to 255 doesn't help (nor do I know why it would). — john01dav, Jan 02 '19 at 20:35
`fout << "P5 " << LENGTH << " " << LENGTH;` Are we sure that this is correct? How does the parser know where the length ends and the data begins? — Lightness Races in Orbit, Jan 02 '19 at 20:35
@UnholySheep There's the error, thanks! :) EDIT: There is still something else wrong. I will edit the question. — john01dav, Jan 02 '19 at 20:35
@LightnessRacesinOrbit That was not correct, but is now corrected in edit. — john01dav, Jan 02 '19 at 20:39
@MarkSetchell My array is a double because the heightmap is calculated as a double. The minimum example uses an array of doubles to have as little change as possible. The data is converted to an unsigned char, however, before it is written to the file. Consider the data in the array to be input to what is being debugged, and its format immutable with respect to what is being debugged. — john01dav, Jan 02 '19 at 20:53
You can't fit a double in an unsigned char - this is not going to work. A NetPBM file can only store unsigned chars between 0..255 or unsigned shorts between 0..65535. If you want to store doubles, you will need to write TIFFs. — Mark Setchell, Jan 02 '19 at 20:54
@MarkSetchell The doubles are all in the range [0, 1]. As such, when they are multiplied by 255 or 256 they are bounded into the range of an unsigned char. C++'s rules for assigning doubles to unsigned chars then convert this range to the representation of an unsigned char. — john01dav, Jan 02 '19 at 20:54
Ok, the `256` in the header is still incorrect- it should be 255. That's why it says `premature end of file` because it is expecting 16 bits per pixel because 256>255. — Mark Setchell, Jan 02 '19 at 20:59
@MarkSetchell That fixed the issue, thanks! I'll accept it if you decide to write it as an answer. — john01dav, Jan 02 '19 at 21:02

score 2 · Accepted Answer · answered Jan 02 '19 at 21:08

Try writing the NetPBM header like this:

fout << "P5\n" << LENGTH << " " << LENGTH << "\n255\n";

The reason is that most image readers check the MAXVAL and if it is 255 or less, they assume 8-bits per pixel. If MAXVAL exceeds 255, they assume 16-bits per pixel, so they will complain that your file is too short (as it doesn't provide 2 bytes per pixel).

What is wrong with my code to write netbpm images?

1 Answers1