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

SGaist

Hi,

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

Christian Ehrlicher

@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

@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

@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

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

abmyii

@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

@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

@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

Beware byteCount is obsolete.

Are you doing something multi-threaded ?

Christian Ehrlicher

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

@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

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

@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

@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

@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

@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

@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

@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

@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!

abmyii

@Christian-Ehrlicher Other than an initial issue:

=================================================================
==463==ERROR: AddressSanitizer: alloc-dealloc-mismatch (operator new vs free) on 0xb1800850
    #0 0xb6af11d7 in free (/usr/lib/arm-linux-gnueabihf/libasan.so.2+0x751d7)
    #1 0xafddfb31  (/usr/lib/arm-linux-gnueabihf/libhybris/linker/mm.so+0x9b31)
    #2 0xafde0b03  (/usr/lib/arm-linux-gnueabihf/libhybris/linker/mm.so+0xab03)
    #3 0xafde1153 in do_dlopen(char const*, int, android_dlextinfo const*) (/usr/lib/arm-linux-gnueabihf/libhybris/linker/mm.so+0xb153)
    #4 0xafddc5bd  (/usr/lib/arm-linux-gnueabihf/libhybris/linker/mm.so+0x65bd)

0xb1800850 is located 0 bytes inside of 33-byte region [0xb1800850,0xb1800871)
allocated by thread T0 here:
    #0 0xb6af1e4b in operator new(unsigned int) (/usr/lib/arm-linux-gnueabihf/libasan.so.2+0x75e4b)
    #1 0xb6fd9ce7  (/lib/ld-linux-armhf.so.3+0x27ce7)

SUMMARY: AddressSanitizer: alloc-dealloc-mismatch ??:0 free
==463==HINT: if you don't care about these warnings you may set ASAN_OPTIONS=alloc_dealloc_mismatch=0
==463==ABORTING

Which goes away by setting the variable it works absolutely fine... Now I'm even more confused...

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

Ah, whoops, my mistake - and what a mistake, haha! That's what happens when you go from Python to C++... So I assume that the malloc sets the vector a new memory range to "look at"?