Access violation in lqr_carver.c

Access violation in lqr_carver.c

annihsman (guest) 09 Jan 2011 22:38

I've gotten this to build in Windows and threw together a simple app using the example code as a guide. When I call lqr_carver_resize I get the error, Access violation reading location, from line 967 in lqr_carver.c.

    if (!r->preserve_in_buffer) {
        g_free(r->rgb); // << This line
    }

I am using the latest build of glib (2.26.1) for windows. If I comment out the line everything continues on fine, however when I scan the pixel data it has not been resized. The image is the size I have requested, but it has been cropped if I resize to a smaller resolution. If I go larger it seems to work fine. Any help would be appreciated.

    LqrCarver *carver = lqr_carver_new(pixs, 32, 32);
    lqr_carver_init(carver, 4, 0); // max_step, rigidity
    lqr_carver_set_energy_function_builtin(carver, LQR_EF_GRAD_XABS);
    lqr_carver_set_side_switch_frequency(carver, 4);
    lqr_carver_set_enl_step(carver, 1.0f);
    lqr_carver_set_resize_order(carver, LQR_RES_ORDER_HOR);
    lqr_carver_resize(carver, 16, 16);
    lqr_carver_scan_reset(carver);

    gint x, y;
    gint w, h;
    guchar *rgb;

    w = lqr_carver_get_width(carver);
    h = lqr_carver_get_height(carver);

    int p = 0;
    while (lqr_carver_scan(carver, &x, &y, &rgb)) {
        /* convert the output into doubles */
        p=(x*bitSize)+((y*bitSize)*w);
        newpixs[p]      = rgb[0];
        newpixs[p+1]    = rgb[1];
        newpixs[p+2]    = rgb[2];

    }

Re: Access violation in lqr_carver.c

CarloB 10 Jan 2011 00:39

The error you get seems to indicate that liblqr is trying to free an invalidly allocated buffer, the one called pixs in your code.
The behaviour you describe seems also to indicate something along that direction, since line 967 is within a function which is only called upon enlargement (and in that case it will try to free the original buffer and substitute it with a larger one, so it's not surprising that the outcome you get when you skip the freeing step is the correct one).

I suggest you double-check:

• if pixs is allocated correctly
• if it is not accidentally freed before being passed over to liblqr
• if there's some other memory corruption event going on elsewhere which may interfere.

I also suggest you use

lqr_carver_set_preserve_input_image(carver);

just after the call to lqr_carver_new, insead of commenting out the troublesome line: that is a setting which implies that you still own the buffer which you pass to lqr_carver_new, so that you will then need to free it yourself at the end, instead of letting liblqr do it).

If you still encounter problems, I guess it may help if you posted the preceding code as well.

annihsman (guest) 10 Jan 2011 02:00

Adding preserve_input_image resolved the Access violation error. I did not realize that was what it was trying to free up. The pixel data is coming from an MFC CBitmap using the GetBitmapBits method. Not sure what sort of hold CBitmap has on its data so it does not get freed, but looks like it is working. The bit of code before what I posted:

unsigned char* pixs = new unsigned char[pixSz];
original->GetBitmapBits(pixSz, pixs);

// ...Much later in code

delete[] pixs;

The other issue of the resize not actually working remains. In the sample code I posted the image comes out as 16x16 as requested, but the image itself is not resized and is instead cropped?

CarloB 10 Jan 2011 02:36

First of all: I just noticed that in your first post, in the call to lqr_carver_new, the channels argument is missing! If that's not a typo, it may lead to undefined behaviour and may be the reason of your problems (but I suppose the compiler would have complained…).

About your last post: from what I can tell by looking at the code, and reading this, it seems that the pixs buffer should be a copy of the original buffer, and therefore that it should be possible to pass it safely to lqr_carver_new and let liblqr to free it, i.e. preserve_input_image shouldn't be necessary.
So there is still some kind of problem there.
Maybe something to do with pixSz (something about the number of color channels, or the color depth maybe)?

Last edited on 10 Jan 2011 02:39 by CarloB Show more

annihsman (guest) 10 Jan 2011 03:00

Oh, yes. My apologies. It is in the code. I changed pieces to have literal values to make it a bit easier to read. It is using 4 channels. Here is the entire block to make life easier. The image is ultimately coming from the clipboard which is where the handle in stgMed is coming from.

If I change this part

new_w = map->bmWidth/2;
new_h = map->bmHeight/2;

to this

new_w = map->bmWidth;
new_h = map->bmHeight;

// or this..

new_w = map->bmWidth*2;
new_h = map->bmHeight*2;

It works as expected with a final result of the exact image or a double sized image. Dividing in half crops.

    CBitmap *original = CBitmap::FromHandle(stgMed.hBitmap);
    CBitmap modified;

    int new_w, new_h;
    BITMAP *map = new BITMAP;
    CBitmap *bitmap = CBitmap::FromHandle(stgMed.hBitmap);
    new_w = map->bmWidth/2;
    new_h = map->bmHeight/2;
    int bitSize = 4;

    int padding = 0;
    while ( (map->bmWidth * bitSize + padding) % bitSize != 0) padding++;
    long pixSz = map->bmHeight*(map->bmWidth*bitSize+padding);
    unsigned char* pixs = new unsigned char[pixSz];
    original->GetBitmapBits(pixSz, pixs);

    padding=0;
    while ( (new_w * bitSize + padding) % bitSize != 0) padding++;
    pixSz = new_h*(new_w*bitSize+padding);

    unsigned char* newpixs = new unsigned char[pixSz];

    LqrCarver *carver = lqr_carver_new(pixs, map->bmWidth, map->bmHeight, bitSize);
    lqr_carver_set_preserve_input_image(carver);
    lqr_carver_init(carver, 4, 0); // max_step, rigidity
    lqr_carver_set_energy_function_builtin(carver, LQR_EF_GRAD_XABS);
    lqr_carver_set_side_switch_frequency(carver, 4);
    lqr_carver_set_enl_step(carver, 1.0f);
    lqr_carver_set_resize_order(carver, LQR_RES_ORDER_HOR);
    lqr_carver_resize(carver, new_w, new_h);
    lqr_carver_scan_reset(carver);

    gint x, y;
    gint w, h;
    guchar *rgb;

    w = lqr_carver_get_width(carver); // RESULT: w = original width/2
    h = lqr_carver_get_height(carver); // RESULT: h = original height/2

    CBitmap bmap;
    int p = 0;
    while (lqr_carver_scan(carver, &x, &y, &rgb)) {
        p=(x*bitSize)+((y*bitSize)*w);
        newpixs[p]        = rgb[0]; 
        newpixs[p+1]    = rgb[1]; 
        newpixs[p+2]    = rgb[2]; 

    }

    modified.CreateBitmap(new_w, new_h, 1, 32, newpixs);

CarloB 10 Jan 2011 03:34

I have 3 observations:

1. I don't get the point of padding, since it seems to me that it will always be 0 (and actually that's fine, since padding/rowstriding is now allowed in lqr_carver_new)
2. it seems you don't read the 4th channel: don't you get a transparent image in that way?
3. from what I understood from the Microsoft documentation (with which I'm not familiar, I must say) the buffers are 2-bytes short integers, not 1-byte unsigned chars. If that is true, you should:
   1. change the type of your buffers and of the rgb pointer to guint16*, and
   2. use lqr_carver_new_ext instead of lqr_carver_new, with LQR_COLDEPTH_16I as the colour detph, and casting the buffer to void*, like this:

LqrCarver * carver = lqr_carver_new_ext((void*) pixs, map->bmWith, map->bmHeight, bitSize, LQR_COLDEPTH_16I);

Edit: I forgot to add to the last point: you should also use lqr_carver_scan_ext, like this:

void * vrgb;
while (lqr_carver_scan_ext(carver, &x, &y, &vrgb)) {
    rgb = (guint16*) vrgb;
    p=(x*bitSize)+((y*bitSize)*w);
    ...
}

Last edited on 10 Jan 2011 04:13 by CarloB Show more

annishman (guest) 10 Jan 2011 19:58

I will give your suggestions a shot when I get home.

In regards to the 4th channel, the image is coming from the Windows Clipboard as a DIB and from what I understand comes in with 4 channels and you don't have much say in the matter, but I could be wrong.

As far as the padding I've worked through the formula with a number of examples and it seems you are correct. This particular sample has been through many changes and at some point without calculating the padding it would not work properly, so it may be unnecessary now. I will need to mess with it.

And don't start me on MS documentation. Sometimes it is brilliant, I'll give them credit where it is due. But sometimes, often?, it is little more than what you get with intellisense and even that leaves a lot to be desired.

CarloB 10 Jan 2011 22:12

Just to be sure: about not reading the 4th channel, I was referring to the scanning portion of the code, towards the end; it seems to me that this line is missing:

newpixs[p+3] = rgb[3];

annihsman (guest) 12 Jan 2011 03:51

Oh, I see. I don't think dropping the alpha was hurting anything. I cleaned up the code a bit and gave your suggestions a try. Switching to guint16* gave me odd results where the resized image is all scrambled up. If I switch everything back to uchar* then I get the same result as before with the cropped image.

CBitmap *original = CBitmap::FromHandle(stgMed.hBitmap);
CBitmap modified;
BITMAP *map = new BITMAP;
int origW, origH, modW, modH, x, y, w, h;
guint16 *rgb;
void *vrgb;
int origChnl = 4;
int modChnl = 4;

original->GetBitmap(map);
origW = map->bmWidth;
origH = map->bmHeight;
modW = origW/2;
modH = origH/2;

long pxSize = map->bmHeight*(map->bmWidth*origChnl);
guint16* origPx = new guint16[pxSize];
original->GetBitmapBits(pxSize, origPx);

pxSize = modH*(modW*modChnl);
guint16* modPx = new guint16[pxSize];

LqrCarver * carver = lqr_carver_new_ext((void*) origPx, origW, origH, origChnl, LQR_COLDEPTH_16I);
//LqrCarver *carver = lqr_carver_new(origPx, origW, origH, origChnl);
lqr_carver_set_preserve_input_image(carver);
lqr_carver_init(carver, 4, 0); // max_step, rigidity
lqr_carver_set_side_switch_frequency(carver, 2);
lqr_carver_set_resize_order(carver, LQR_RES_ORDER_HOR);
lqr_carver_resize(carver, modW, modH);
lqr_carver_scan_reset(carver);

w = lqr_carver_get_width(carver);
h = lqr_carver_get_height(carver);
int p = 0;

while (lqr_carver_scan_ext(carver, &x, &y, &vrgb)) {
    rgb = (guint16*) vrgb;
    p=(x*(modChnl))+((y*(modChnl))*w);
    modPx[p]    = rgb[0];
    if(modChnl>1) modPx[p+1]    = rgb[1];
    if(modChnl>2) modPx[p+2]    = rgb[2];
    if(modChnl>3) modPx[p+3]    = rgb[3];
}

// It seems to calc the overall bits I should be multiplying by 16
// instead of 8, but if I do that it creates a blank bitmap.
modified.CreateBitmap(modW, modH, 1, modChnl*8, modPx);

// Cleanup
lqr_carver_destroy(carver);
delete[] origPx;
delete[] modPx;
...

CarloB 25 Jan 2011 14:12

I don't know, it still seems to me that the buffers should be short-integers rather then uchars, see e.g. here:

lpBits
Points to a short-integer array that contains the initial bitmap bit values.

and here:

lpBits
Points to the buffer that is to receive the bitmap. The bitmap is an array of bytes. The bitmap byte array conforms to a structure where horizontal scan lines are multiples of 16 bits.

and here:

bmWidthBytes
Specifies the number of bytes in each raster line. This value must be an even number since the graphics device interface (GDI) assumes that the bit values of a bitmap form an array of integer (2-byte) values. In other words, bmWidthBytes * 8 must be the next multiple of 16 greater than or equal to the value obtained when the bmWidth member is multiplied by the bmBitsPixel member.

Solving or at least fully understanding this issue seems crucial to me.

annishman (guest) 27 Jan 2011 02:45

I agree. I suppose I am a bit stuck at this part myself. It does appear to be working somewhat but I can't seem to work out where I am wrong calculating things up. Maybe seeing an example will help spot what I am missing. From the above code I see the following result. Left is the source image, right the result. It appears that every other scan line is shifted over, but I do not understand why.

I believe it is my pixel calculation in the loop, but I've run it through a number of times and it seems like it should be right. I admit these sorts of formulas are not my strength.

p=(x*(modChnl))+((y*(modChnl))*w);

Re: Access violation in lqr_carver.c

CarloB 27 Jan 2011 15:32

(I'm starting over without replying because we reached the replies limit of the forum.)

The formula which computes p seems correct to me.

Looking at the image, I'd rather think about some kind of padding problem.

One thing I found is that you should probably change this line:

original->GetBitmapBits(pxSize, origPx);

like this:

original->GetBitmapBits(pxSize * 2, origPx);

because accrding to the documentation the first argument

Specifies the number of bytes to be copied.

and each channel has 2 bytes per pixel.

More random suggestions:

• check the value of orig->bmBitsPixel: it should be equal to 4 * 16 = 64 (channels * bits per channel).
• check if orig->bmWidth * 4 * 2 is equal to orig->bmWidthBytes (if it is not, there may be a padding issue)
• use modChnl * 16 in CreateBitmap
• maybe try to avoid the liquidrescale step and see whether you're able to get good results. You may try to do your own simple modifications to the image buffer, just to be sure that you undersstood the structure of the buffer: for example
  • double the size of the buffer and replicate the image four times, or
  • halve the size and pick one pixel out of four (or an average), or
  • invert two colour channels; and so on.
• find whoever wrote the Microsoft documentation and sue them for crimes against humanity :) (ok, just kidding, but really, I never saw anything like it)