Dump QImage raw pixel data into `std::vector<char>`

abmyii · wrote on 3 Dec 2020, 18:35

@Christian-Ehrlicher The vector is the same size as the image. Nothing jumps out at me in the GDB logs (though I have very little experience with it).

I find it curious that it works for a while and then stops. Perhaps the memcpy fails because QImage data isn't contiguous in memory? Also surely memcpying isn't an ideal solution anyway?

SGaist · 3 Dec 2020, 18:46

Hi,

Can you show how you resize your vector ?
Are the image always the same ?

Christian Ehrlicher · 3 Dec 2020, 19:10

@abmyii said in Dump QImage raw pixel data into `std::vector<char>`:

QImage data isn't contiguous in memory?

It is and memcpy is fine.

Use valgrind or address sanitizer to see if you have some out-of-bounds access.

abmyii · S SGaist 3 Dec 2020, 18:38

@SGaist Hello!

The vector remains the same size throughout, and so does the image. The program is a VNC server so the sizes are constant. I do rotate the image though at times, but the dimensions are just swapped so the actual data is the same size I believe.

    auto const frame_size_bytes = rgba_pixel_size *
                                  size.width.as_uint32_t() *
                                  size.height.as_uint32_t();

    std::vector<char> frame_data(frame_size_bytes, 0);

Christian Ehrlicher · 12 Mar 2020, 18:51

@abmyii said in Dump QImage raw pixel data into `std::vector<char>`:

size I believe.

Why not check instead assuming something?

/edit: @SGaist is correct - you forgot the padding

SGaist · 3 Dec 2020, 18:57

You should rather use QImage::sizeInBytes. That should take into account any padding depending on the format you are using.

abmyii · S SGaist 3 Dec 2020, 18:49

@SGaist Hmm, I'm not converting it to any format. Also I get error: ‘class QImage’ has no member named ‘sizeInBytes’ so I can't check if any padding is added regardless - I think the device has Qt 5.9 on it.

This is the code that makes and transforms the QImage:

    QImage image(width, height, QImage::Format_RGBA8888); 
    glReadPixels(0, 0, width, height, format, GL_UNSIGNED_BYTE, image.bits()); 
 
    if (mirror) 
        image = image.mirrored(true, true); 
 
    if (landscape) { 
        //QPoint center = image.rect().center(); 
        QMatrix matrix; 
        matrix.translate(0, 0); 
        matrix.rotate(90); 
 
        image = image.transformed(matrix); 
    } 

    std::memcpy(fb, (const char *)image.constBits(), image.byteCount());

Christian Ehrlicher · wrote on 3 Dec 2020, 19:05

@abmyii said in Dump QImage raw pixel data into `std::vector<char>`:

o I can't check if any padding is added regardless - I think the device has Qt 5.9 on it.

You already use it, although with another name...

std::memcpy(fb, (const char *)image.constBits(), image.byteCount());

abmyii · wrote on 3 Dec 2020, 19:10

@Christian-Ehrlicher With valgrind it didn't crash, but also the program was a lot slower so perhaps when it runs at normal speed the issue occurs?

And yup, as for the byteCount, that's what I was referring to when I said I checked the sizes. image.byteCount() and frame_buffer.size().

(Replying to 2 in 1 because of the 600s posting limit...)

BTW thank you all for the speedy suggestions/replies!

SGaist · 3 Dec 2020, 19:25

Beware byteCount is obsolete.

Are you doing something multi-threaded ?

Christian Ehrlicher · wrote on 3 Dec 2020, 19:15

In your first post you use a std::vector<char>() as container, in your last post a plain pointer. How do you allocate this memory?

valgrind is slow, yes - therefore I suggested AdressSanitizer or let it run for an hour and see what happens with valgrind.

abmyii · S SGaist 3 Dec 2020, 19:15

@SGaist said in Dump QImage raw pixel data into `std::vector<char>`:

Beware byteCount is obsolete.

Are you doing something multi-threaded ?

Yes, but not the image -> fb conversion.

@Christian-Ehrlicher said in Dump QImage raw pixel data into `std::vector<char>`:

In your first post you use a std::vector<char>() as container, in your last post a plain pointer. How do you allocate this memory?

valgrind is slow, yes - therefore I suggested AdressSanitizer or let it run for an hour and see what happens with valgrind.

The memcpy is in a function, and fb is a pointer to frame_data.data() as in the original example.

The full code is hosted on GitLab (https://gitlab.com/abmyii/ubports-mir-vnc-server/-/blob/e3be9585007ea2b663139fd2f5345745392cf9c7/mirvncserver.cpp) but is quite messy. The relevant bits are:

https://gitlab.com/abmyii/ubports-mir-vnc-server/-/blob/e3be9585007ea2b663139fd2f5345745392cf9c7/mirvncserver.cpp#L94-123 (The function for reading the pixels)
https://gitlab.com/abmyii/ubports-mir-vnc-server/-/blob/e3be9585007ea2b663139fd2f5345745392cf9c7/mirvncserver.cpp#L586-591 (The frame buffer initialisation)

https://gitlab.com/abmyii/ubports-mir-vnc-server/-/blob/e3be9585007ea2b663139fd2f5345745392cf9c7/mirvncserver.cpp#L619 (The function call)

There is a chance that the Bus Error is occurring at the VNC level though...

abmyii · 12 Mar 2020, 21:54

I'm trying (and failing) to get it compiled with Clang + AddressSanitizer (linking errors - probably not converting from GCC correctly), but I was thinking - is there any reasonably fast method to copy the pixels without using malloc just to ensure that it is(n't) the malloc causing the issue?

Christian Ehrlicher · 4 Dec 2020, 10:06

@abmyii said in Dump QImage raw pixel data into `std::vector<char>`:

copy the pixels without using malloc just to ensure that it is(n't) the malloc causing the issue?

What should be faster? When you allocate enough pixels (which you still did not check, at least in your code we've seen until now) than all is fine. Otherwise also valgrind would have told you.

abmyii · 4 Dec 2020, 12:00

@Christian-Ehrlicher What I mean is that are there other methods that don't require malloc? I want to try those to see if using those methods causes the crashes to stop. As for allocating enough pixels I checked that some time ago and then removed the code but I've added it back in with this output (doesn't change throughout the execution of the program):

FB size: 1327104, Image byteCount: 1327104

Valgrind didn't crash (unfortunately) but there is obviously an issue somewhere, so now finding it isn't going to be easy. I do believe it's on the VNC side, not Qt, but is being induced somehow by the malloc.

Compiling with Clang instead of G++ isn't straightforward for me at my level of knowledge. On compiling I get the following warnings:

clang mirvncserver.cpp -c -std=c++11 -Wall -fpermissive -I/usr/include/mirclient -I/usr/include/mircommon -I/usr/include/mircore -I/usr/include/libevdev-1.0 -I/usr/include/arm-linux-gnueabihf/qt5 -fPIC -fsanitize=address -O1 -fno-omit-frame-pointer -g -o mirvncserver-clang -lboost_program_options -lpthread -lmirclient -lEGL -lxcb-glx -lGLESv2 -lmirserver -lmircore -levdev -lvncserver -lstdc++ -lQt5Gui -lQt5Core

clang: warning: -lboost_program_options: 'linker' input unused
clang: warning: -lpthread: 'linker' input unused
clang: warning: -lmirclient: 'linker' input unused
clang: warning: -lEGL: 'linker' input unused
clang: warning: -lxcb-glx: 'linker' input unused
clang: warning: -lGLESv2: 'linker' input unused
clang: warning: -lmirserver: 'linker' input unused
clang: warning: -lmircore: 'linker' input unused
clang: warning: -levdev: 'linker' input unused
clang: warning: -lvncserver: 'linker' input unused
clang: warning: -Z-reserved-lib-stdc++: 'linker' input unused
clang: warning: -lQt5Gui: 'linker' input unused
clang: warning: -lQt5Core: 'linker' input unused

And the output file is not executable mirvncserver-clang: ELF 32-bit LSB relocatable, ARM, EABI5 version 1 (SYSV), not stripped. I'm sure I'm making a very basic error, but I'm not sure what it is.

Christian Ehrlicher · 4 Dec 2020, 13:47

@abmyii said in Dump QImage raw pixel data into `std::vector<char>`:

want to try those to see if using those methods causes the crashes to stop

You're aware that all functions which allocate memory is using malloc (C) or new (C++)? So what do you think you gain. If your allocated enough memory than all is fine. Otherwise use a debugger and examine your stack trace

Why do you want to compile your program with clang now? I said you should use the Address Sanitizer - no need to use clang.

abmyii · wrote on 4 Dec 2020, 13:47

@Christian-Ehrlicher said in Dump QImage raw pixel data into `std::vector<char>`:

@abmyii said in Dump QImage raw pixel data into `std::vector<char>`:

want to try those to see if using those methods causes the crashes to stop

You're aware that all functions which allocate memory is using malloc (C) or new (C++)? So what do you think you gain. If your allocated enough memory than all is fine. Otherwise use a debugger and examine your stack trace

Why do you want to compile your program with clang now? I said you should use the Address Sanitizer - no need to use clang.

Ah, according to the instructions in the repo you must compile with clang (https://github.com/google/sanitizers/wiki/AddressSanitizer#using-addresssanitizer). How can I do that with G++?

Rather than mallocing I was thinking I could push_back or set the pixel data using something like fb[pos] = (char)qRed(pixel), but I would definitely prefer malloc if I can find the issue...

Christian Ehrlicher · 4 Dec 2020, 15:57

@abmyii said in Dump QImage raw pixel data into `std::vector<char>`:

Rather than mallocing I was thinking I could push_back

Ok, please re-read and think over again. malloc and push_back don't have anything in common.

asan: http://stackoverflow.com/questions/37970758/ddg#40215639 for example

abmyii · wrote on 4 Dec 2020, 15:57

@Christian-Ehrlicher said in Dump QImage raw pixel data into `std::vector<char>`:

Ok, please re-read and think over again. malloc and push_back don't have anything in common.

Sorry, perhaps I'm misunderstanding but that is the whole point - I want to test if using a different method other than malloc for copying the data will fix the issue.

Thank you for the link, I'll try that now.

Christian Ehrlicher · wrote on 4 Dec 2020, 16:35

@abmyii said in Dump QImage raw pixel data into `std::vector<char>`:

other than malloc for copying the data will fix the issue.

Again: malloc does not copy anything. It allocates memory!