2

I am writing a program which will preform texture synthesis. I have been away from C++ for a while and am having trouble figuring out what I am doing wrong in my class. When I run the program, I get an unhandled exception in the copyToSample function when it tries to access the arrays. It is being called from the bestSampleSearch function when the unhandled exception occurs. The function has been called before and works just fine, but later on in the program it is called a second time and fails. Any ideas? Let me know if anyone needs to see more code. Thanks!

Edit1: Added the bestSampleSearch function and the compareMetaPic function
Edit2: Added a copy constructor
Edit3: Added main()
Edit4: I have gotten the program to work. However there is now a memory leak of some kind or I am running out of memory when I run the program. It seems in the double for loop in main which starts "// while output picture is unfilled" is the problem. If I comment this portion out the program finishes in a timely manner but only one small square is output. Something must be wrong with my bestSampleSearch function.

MetaPic.h

#pragma once
#include <pic.h>
#include <stdlib.h>
#include <cmath>

class MetaPic
{
    public:
        Pic* source;
        Pixel1*** meta;
        int x;
        int y;
        int z;
        MetaPic();
        MetaPic(Pic*);
        MetaPic(const MetaPic&);
        MetaPic& operator=(const MetaPic&);
        ~MetaPic();
        void allocateMetaPic();
        void copyPixelData();
        void copyToOutput(Pic*&);
        void copyToMetaOutput(MetaPic&, int, int);
        void copyToSample(MetaPic&, int, int);
        void freeMetaPic();

};

MetaPic.cpp

#include "MetaPic.h"

MetaPic::MetaPic()
{
    source = NULL;
    meta = NULL;
    x = 0;
    y = 0;
    z = 0;
}

MetaPic::MetaPic(Pic* pic)
{
    source = pic;
    x = pic->nx;
    y = pic->ny;
    z = pic->bpp;
    allocateMetaPic();
    copyPixelData();
}

MetaPic::MetaPic(const MetaPic& mp)
{
    source = mp.source;
    x = mp.x;
    y = mp.y;
    z = mp.z;
    allocateMetaPic();
    copyPixelData();
}

MetaPic::~MetaPic()
{
    freeMetaPic();
}

// create a 3 dimensional array from the original one dimensional array
void MetaPic::allocateMetaPic()
{
    meta = (Pixel1***)calloc(x, sizeof(Pixel1**));

    for(int i = 0; i < x; i++)
    {
        meta[i] = (Pixel1**)calloc(y, sizeof(Pixel1*));
        for(int j = 0; j < y; j++)
        {
            meta[i][j] = (Pixel1*)calloc(z, sizeof(Pixel1));
        }
    }
}

void MetaPic::copyPixelData()
{
    for(int j = 0; j < y; j++)
    {
        for(int i = 0; i < x; i++)
        {
            for(int k = 0; k < z; k++)
                meta[i][j][k] = source->pix[(j*z*x)+(i*z)+k];
        }
    }
}

void MetaPic::copyToOutput(Pic* &output)
{
    for(int j = 0; j < y; j++)
    {
        for(int i = 0; i < x; i++)
        {
            for(int k = 0; k < z; k++)
                output->pix[(j*z*x)+(i*z)+k] = meta[i][j][k];
        }
    }
}

// copy the meta data to the final  pic output starting at the top left of the picture     and mapped to 'a' and 'b' coordinates in the output
void MetaPic::copyToMetaOutput(MetaPic &output, int a, int b)
{
    for(int j = 0; (j < y) && ((j+b) < output.y); j++)
    {
        for(int i = 0; (i < x) && ((i+a) < output.x); i++)
        {
            for(int k = 0; k < z; k++)
                output.meta[i+a][j+b][k] = meta[i][j][k];
        }
    }
}

// copies from a source image to a smaller sample image
// *** Must make sure that the x and y coordinates have enough buffer space ***
void MetaPic::copyToSample(MetaPic &sample, int a, int b)
{
    for(int j = 0; (j < sample.y) && ((b+j) < y); j++)
    {
        for(int i = 0; i < (sample.x) && ((a+i) < x); i++)
        {
            for(int k = 0; k < sample.z; k++)
            {
                    **sample.meta[i][j][k] = meta[i+a][j+b][k];**
            }
        }
    }
}

// free the meta pic data (MetaPic.meta)
// *** Not to be used outside of class declaration ***
void MetaPic::freeMetaPic()
{
    for(int j = 0; j < y; j++)
    {
        for(int i = 0; i < z; i++)
            free(meta[i][j]);
    }
    for(int i = 0; i < x; i++)
        free(meta[i]);

    free(meta);
}

MetaPic MetaPic::operator=(MetaPic mp)
{
    MetaPic newMP(mp.source);

    return newMP;
}

main.cpp

#ifdef WIN32
// For VC++ you need to include this file as glut.h and gl.h refer to it
#include <windows.h>
// disable the warning for the use of strdup and friends
#pragma warning(disable:4996) 
#endif
#include <stdio.h>     // Standard Header For Most Programs
#include <stdlib.h>    // Additional standard Functions (exit() for example)
#include <iostream>
// Interface to libpicio, provides functions to load/save jpeg files
#include <pic.h>
#include <string.h>
#include <time.h>
#include <cmath>

#include "MetaPic.h"

using namespace std;

MetaPic bestSampleSearch(MetaPic, MetaPic);
double compareMetaPics(MetaPic, MetaPic);

#define SAMPLE_SIZE 23
#define OVERLAP 9

// Texture source image (pic.h uses the Pic* data structure)
Pic *sourceImage;
Pic *outputImage;
int main(int argc, char* argv[])
{
    char* pictureName = "reg1.jpg";
    int outputWidth = 0;
    int outputHeight = 0;

    // attempt to read in the file name
    sourceImage = pic_read(pictureName, NULL);
    if(sourceImage == NULL)
    {
        cout << "Couldn't read the file" << endl;
        system("pause");
        exit(EXIT_FAILURE);
    }

    // *** For now set the output image to 3 times the original height and width ***
    outputWidth = sourceImage->nx*3;
    outputHeight = sourceImage->ny*3;

    // allocate the output image
    outputImage = pic_alloc(outputWidth, outputHeight, sourceImage->bpp, NULL);
    Pic* currentImage = pic_alloc(SAMPLE_SIZE, SAMPLE_SIZE, sourceImage->bpp, NULL);

    MetaPic metaSource(sourceImage);
    MetaPic metaOutput(outputImage);
    MetaPic metaCurrent(currentImage);

    // seed the output image    
    int x = 0;
    int y = 0;
    int xupperbound = metaSource.x - SAMPLE_SIZE;
    int yupperbound = metaSource.y - SAMPLE_SIZE;
    int xlowerbound = 0;
    int ylowerbound = 0;

    // find random coordinates
    srand(time(NULL));
    while((x >= xupperbound) || (x <= xlowerbound))
        x = rand() % metaSource.x;
    while((y >= yupperbound) || (y <= ylowerbound))
        y = rand() % metaSource.y;

    // copy a random sample from the source to the metasample
    metaSource.copyToSample(metaCurrent, x, y);
    // copy the seed to the metaoutput
    metaCurrent.copyToMetaOutput(metaOutput, 0, 0);


    int currentOutputX = 0;
    int currentOutputY = 0;

    // while the output picture is unfilled...
    for(int j = 0; j < yupperbound; j+=(SAMPLE_SIZE-OVERLAP))
    {
        for(int i = 0; i < xupperbound; i+=(SAMPLE_SIZE-OVERLAP))
        {
            // move the sample to correct overlap
            metaSource.copyToSample(metaCurrent, i, j);
            // find the best match for the sample
            metaCurrent = bestSampleSearch(metaSource, metaCurrent);
            // write the best match to the metaoutput
            metaCurrent.copyToMetaOutput(metaOutput, i, j);
            // update the values
        }
    }


    // copy the metaOutput to the output
    metaOutput.copyToOutput(outputImage);

    // output the image
    pic_write("reg1_output.jpg", outputImage, PIC_JPEG_FILE);


    // clean up
    pic_free(sourceImage);
    pic_free(outputImage);
    pic_free(currentImage);

    // return success
    cout << "Done!" << endl;
    system("pause");
    // return success
    return 0;
}

// finds the best sample to insert into the image
// *** best must be the sample which consists of the overlap ***
MetaPic bestSampleSearch(MetaPic source, MetaPic best)
{
    MetaPic metaSample(best);

    double bestScore = 999999.0;
    double currentScore = 0.0;

    for(int j = 0; j < source.y; j++)
    {
        for(int i = 0; i < source.x; i++)
        {
            // copy the image starting at the top left of the source image
            source.copyToSample(metaSample, i, j);
            // compare the sample with the overlap
            currentScore = compareMetaPics(best, metaSample);
            // if best score is greater than current score then copy the         better sample to best and continue searching
            if( bestScore > currentScore)
            {
                metaSample.copyToSample(best, 0, 0);
                bestScore = currentScore;
            }
            // otherwise, the score is less than current score then do nothing     (a better sample has not been found)

        }
    }

    return best;
}

// find the comparison score for the two MetaPics based on their rgb values
// *** Both of the meta pics should be the same size ***
double compareMetaPics(MetaPic pic1, MetaPic pic2)
{
    float r1 = 0.0;
    float g1 = 0.0;
    float b1 = 0.0;
    float r2 = 0.0;
    float g2 = 0.0;
    float b2 = 0.0;
    float r = 0.0;
    float g = 0.0;
    float b = 0.0;

    float sum = 0.0;

    // take the sum of the (sqrt((r1-r2)^2 + ((g1-g2)^2 + ((b1-b2)^2))
    for(int j = 0; (j < pic1.y) && (j < pic2.y); j++)
    {
        for(int i = 0; (i < pic1.x) && (i < pic2.x); i++)
        {
            r1 = PIC_PIXEL(pic1.source, i, j, 0);
            r2 = PIC_PIXEL(pic2.source, i, j, 0);
            g1 = PIC_PIXEL(pic1.source, i, j, 1);
            g2 = PIC_PIXEL(pic2.source, i, j, 1);
            b1 = PIC_PIXEL(pic1.source, i, j, 2);
            b2 = PIC_PIXEL(pic2.source, i, j, 2);

            r = r1 - r2;
            g = g1 - g2;
            b = b1 - b2;

            sum += sqrt((r*r) + (g*g) + (b*b));
        }
    }

    return sum;
}
please delete me
  • 137
  • 1
  • 1
  • 7
  • 3
    calloc and C++ and multidimensional arrays make my eyes bleed! Also there can be any number of things wrong here. Can you post like 10 lines which reproduce the problem? http://sscce.org/ – FailedDev Dec 05 '11 at 22:35
  • 2
    ¤ Missing info: you're not showing the class definition. But apparently you have neither supported nor disabled *copying*, and then you risk that e.g. your pointers are copied raw over into another object, and then that two (or more) objects think they own them and have to delete them... Check out the C++ Law of Three™ about this. That said, a few weeks ago I learned a new term for the kind of code you present, that it's Three Star Code™, i.e. that it contains `***`. That indicates extreme Wrongness™. Use e.g. `std::vector` instead, or other standard library container. Cheers & hth., – Cheers and hth. - Alf Dec 05 '11 at 22:40
  • @FailedDev It would be difficult to get it down in 10 lines. I posted the bestSampleSearch function which is as close as I can get. My main which uses all these functions is ~130 lines. – please delete me Dec 05 '11 at 22:40
  • 1
    @MJPiarulli Take heed at what Alf said. You are most probably copying some object while forgeting to copy the data pointed by meta. Define a copy construcor and an assignment operator and try to copy/assign/destroy objects. If that works you are in a good road :) – FailedDev Dec 05 '11 at 22:47
  • @AlfP.Steinbach: It is the class *declaration* which is missing, but your point is certainly valid. – e.James Dec 05 '11 at 22:49
  • 1
    @e.James: (1) no, it is the class *definition*, (2) of course my point is valid, the general doubt about my points that you indicate here does not exist, and (3) when you're going to quibble about terminology, not only make certain you're right, which you forgot to do, but do also provide a reference. In this case a proper reference would be the Holy Standard. The current standard, C++11, gives an *example* of a "class definition" in §9.1./1 and also in §9.2/13. The latter is perhaps easier to understand, as there is no question of what exactly constitutes the definition. Cheers & hth., – Cheers and hth. - Alf Dec 05 '11 at 22:59
  • Also, I don't see the `main()` function, and therefore don't know what functions are being called when. – David Thornley Dec 05 '11 at 23:02
  • @David Thornley I left out the main() to save on space. I'll post it if you would like to see it though. – please delete me Dec 05 '11 at 23:04
  • @AlfP.Steinbach: I will happily concede your point that the standard uses the term *class definition* in this manner. I have always seen (until just now) the list of class data members and class methods (i.e. the part that reads `class X { ... };` described as its *declaration*, and the implementation of the class methods described as the *definition*. I am now not sure which way is correct. If you use *definition* in the manner you suggest, what term do you use to describe the parts that (usually) end up in the source file? – e.James Dec 05 '11 at 23:09
  • @AlfP.Steinbach: as far as "quibbling" goes, I certainly did not mean to cause you any personal offense. We obviously have different definitions for the same term, and I am eager to determine which term is correct. – e.James Dec 05 '11 at 23:11
  • It seems the sample.meta array is working correctly now. However the meta array on the right side of the = is still not being handled properly. Progress is being made here. – please delete me Dec 05 '11 at 23:13
  • @e.James: A definition is also a declaration, hence it's not incorrect to refer to a class definition as a declaration. A member definition is a member definition. A class definition has to declare the class members but but does not have to define them. A pure class declaration, such as `struct MyClass;`, does not even declare the members. It yields an incomplete type, one that you cannot use with `sizeof`, unless the class has already been defined. – Cheers and hth. - Alf Dec 05 '11 at 23:13
  • @Alf P. Steinbach Could you explain more how the three star code is considered "extreme wrongness"? This is how you would do it in standard c, right? Or is vector or some container declared in stdlib now instead? – please delete me Dec 06 '11 at 00:04
  • 1
    @MJPiarulli: In C you don't have the abstractions that you do have in C++. So in C you can be forced to do things in ways that from a C++ point of view are ungood. Doing that kind of C style coding in C++ is akin to doing something ordinary in inline assembly language in C, unnecessarily going down to a lower abstraction level language. `std::vector` is defined in the `vector` header. It takes care of allocation and deallocation and copying, and even, if you want, resizing. It does all this automatically. :-) – Cheers and hth. - Alf Dec 06 '11 at 00:18
  • Ideally, you'd reduce the problem to relatively few lines of code. If you can't do that, giving us something that will compile and fail to run will at least let us try to find what's wrong. It's hard to look at a lot of code that's missing class definitions and order of calls and figure something out. – David Thornley Dec 06 '11 at 00:26
  • @David Thornley Okay, I have separated all of the code into their respective files. Should be easier to read and compile now. – please delete me Dec 06 '11 at 02:25

2 Answers2

0

I'm not sure if this is the root cause of the problem, but your assignment operator does not actually assign anything:

MetaPic MetaPic::operator=(MetaPic mp)
{
    MetaPic newMP(mp.source);

    return newMP;
}

This should probably look something like the following (based off of the code in your copy constructor):

edit: with credit to Alf P. Steinbach

MetaPic& MetaPic::operator=(MetaPic mp)
{
    mp.swap(*this);
    return *this;
}
Community
  • 1
  • 1
e.James
  • 116,942
  • 41
  • 177
  • 214
  • Thank you for the fix. The overall problem still exists though. – please delete me Dec 05 '11 at 22:49
  • 4
    -1 unnecessarily complex and not exception safe. The common idiom for this kind of assignment operator is `MetaPic& operator=( MetaPic other ) { other.swap( *this ); }`. It's known as the **swap idiom**, and it works by letting the copy constructor do the copying. Never implement copy construction in terms of assignment. Do implement assignment in terms of copy construction. Although in this case, the best is to use `std::vector` (or similar) and not have to implement copy constructor and assignment operator. – Cheers and hth. - Alf Dec 05 '11 at 22:51
  • @AlfP.Steinbach: I like the trick of letting the copy constructor do the copying. I assume you would still end with `return *this;`? – e.James Dec 05 '11 at 22:56
0

It turns out that the deallocate function is incorrect. It should be freeing in the same manner that it was allocating.

void MetaPic::freeMetaPic()

{

    for(int j = 0; j < y; j++)

    {

        for(int i = 0; i < z; i++)

            free(meta[i][j]);

    }

    for(int i = 0; i < x; i++)

        free(meta[i]);



    free(meta);

}
please delete me
  • 137
  • 1
  • 1
  • 7