alliedvision / VimbaPython

Old Allied Vision Vimba Python API. The successor to this API is VmbPy
BSD 2-Clause "Simplified" License
93 stars 40 forks source link

Streaming FrameHandler crashes when using Frame.convert_pixel_format() #114

Open te-bachi opened 2 years ago

te-bachi commented 2 years ago

I started with the development of a GUI application with PySide2 together with VimbaPython using PixelFormat RGB8. I did 72h tests capturing RGB8 frames with 12 fps without any problems.

After switching to BayerRG12 the FrameHandler processes 5 correct frames, but at the 6th frame the conversion is faulty, probably because of frame.convert_pixel_format(PixelFormat.Rgb8) and after the 9th frame, no frames were captured anymore and sometimes the application crashed.

Is this a bug or how can I deal with it?

Success Faulty

FrameHandler

from PySide2.QtCore import Signal, Slot, QThread
from PySide2.QtGui import QImage

from vimba import Vimba, Camera, Frame, VimbaFeatureError, FrameStatus, PixelFormat, VimbaCameraError

class VimbaThread(QThread):

    onFrameReceived = Signal(QImage)
    onResponseFeaturesError = Signal(str)

    def __init__(self, parent=None):
        super().__init__(parent)
        self.__cam = None

    [...]

    def __frameHandler(self, cam: Camera, frame: Frame):
        if frame.get_status() == FrameStatus.Complete:
            pixelFormat = frame.get_pixel_format()

            if pixelFormat == PixelFormat.Rgb8:
                image = QImage(frame.get_buffer(), frame.get_width(), frame.get_height(), QImage.Format_RGB888)
            elif pixelFormat == PixelFormat.BayerRG8 or pixelFormat == PixelFormat.BayerRG12:
                frame.convert_pixel_format(PixelFormat.Rgb8)
                image = QImage(frame.get_buffer(), frame.get_width(), frame.get_height(), QImage.Format_RGB888)
            else:
                image = QImage(frame.get_buffer(), frame.get_width(), frame.get_height(), QImage.Format_Grayscale8)

            self.onFrameReceived.emit(image)

        cam.queue_frame(frame)

    def run(self) -> None:
        try:
            with Vimba.get_instance() as vimba:
                with vimba.get_camera_by_id(self.__cam.get_id()) as cam:

                    [...]
                    cam.set_pixel_format(PixelFormat.BayerRG12)
                    cam.start_streaming(handler=self.__frameHandler)

                    # QThread.exec_() enters event loop
                    self.exec_()

                    cam.stop_streaming()

        except VimbaCameraError as err:
            self.onResponseFeaturesError.emit(str(err))

Environment

Allied Vision 1800 U-511C-CH-C
conda       4.12.0
python      3.10.4
vimbapython 1.1.0
pyside2     5.13.2
te-bachi commented 2 years ago

I checked the RAM usage from both PixelFormats.

RGB8

Only uses frame.get_buffer() It just allocates once a buffer: RAM Usage BayerRG8

BayerRG8

Uses frame.convert_pixel_format(PixelFormat.Rgb8) before frame.get_buffer() It increases every time a frame is received (6th frame faulty, 9th/10th frame crash of FrameHandler): RAM Usage BayerRG8

Is this a python bug or a Vimba SDK bug?

te-bachi commented 2 years ago

Ah, there is a new version of Vimba SDK. I downloaded it long time ago on 07.10.2021, so it was Vimba SDK 5.0. I'll check the newest version Vimba SDK 6.0 and give feedback...

te-bachi commented 2 years ago

Ok, same in Vimba SDK 6.0, faulty on 6th frame, crash of FrameHandler on 13th frame, Python application continues to run normaly, no error or exception:

RAM usage for Vimba SDK 6.0

Environment:

Allied Vision 1800 U-511C-CH-C
Vimba SDK 6.0
Windows 10 21H2 
conda 4.13.0
python 3.10.5
vimbapython 1.2.1
pyside2 5.15.5
NiklasKroeger-AlliedVision commented 2 years ago

Frame.convert_pixel_format currently works, by converting the frame object in-place. That means, that when you call the method, the allocated frame buffer is passed on to our Image Transform library, and the result from that transformation is written to a new buffer. That new buffer then replaces the original buffer and metadata is updated accordingly. The original buffer is removed from the Frame instance and Pythons garbage collector will (eventually) free the associated memory.

This means, that once convert_pixel_format is called on an instance of Frame, that object is changed in such a way, that it should ideally not be reused for frame transmissions. The reason is that for some conversions, the size of the buffer might actually have been reduced and subsequent frame transmissions would try to write to invalid memory addresses beyond the buffers boundaries. For cases where the newly allocated buffer is larger than the payload data that should be transmitted, the required memory size will increase unnecessarily as you have observed in your screenshot shared for the BayerRG8 case. In most cases frame transmission will still work, but subsequent calls to convert_pixel_format could lead to unexpected behaviour (I have not thoroughly investigated this yet).

I hope this explains the reason for this issue. Now for a possible solution.

In your frame handler you are taking the original frame you received from the camera, and call convert_pixel_format on it (at least for the case where you are experiencing issues). Afterwards, you requeue the frame for the next transmission. As described above, the pixel format conversion changes the buffer size and some internal metadata of the Frame instance. To prevent this, I would recommend the following:

import copy  # needed for calls to copy.deepcopy to create a definitive copy of the received frame

from PySide2.QtCore import Signal, Slot, QThread
from PySide2.QtGui import QImage

from vimba import Vimba, Camera, Frame, VimbaFeatureError, FrameStatus, PixelFormat, VimbaCameraError

class VimbaThread(QThread):

    onFrameReceived = Signal(QImage)
    onResponseFeaturesError = Signal(str)

    def __init__(self, parent=None):
        super().__init__(parent)
        self.__cam = None

    [...]

    def __frameHandler(self, cam: Camera, frame: Frame):
        if frame.get_status() == FrameStatus.Complete:
            pixelFormat = frame.get_pixel_format()

            if pixelFormat == PixelFormat.Rgb8:
                image = QImage(frame.get_buffer(), frame.get_width(), frame.get_height(), QImage.Format_RGB888)
            elif pixelFormat == PixelFormat.BayerRG8 or pixelFormat == PixelFormat.BayerRG12:
                # A copy of the frame is created because `convert_pixel_format` changes the instance
                # in-place and therefore makes reusing the object for further transmissions
                # problematic
                frame_cpy = copy.deepcopy(frame)
                frame_cpy.convert_pixel_format(PixelFormat.Rgb8)
                # Use the copied frame instance to get the converted pixel data
                image = QImage(frame_cpy.get_buffer(), frame_cpy.get_width(), frame_cpy.get_height(), QImage.Format_RGB888)
            else:
                image = QImage(frame.get_buffer(), frame.get_width(), frame.get_height(), QImage.Format_Grayscale8)

            self.onFrameReceived.emit(image)

        cam.queue_frame(frame)

    def run(self) -> None:
        try:
            with Vimba.get_instance() as vimba:
                with vimba.get_camera_by_id(self.__cam.get_id()) as cam:

                    [...]
                    cam.set_pixel_format(PixelFormat.BayerRG12)
                    cam.start_streaming(handler=self.__frameHandler)

                    # QThread.exec_() enters event loop
                    self.exec_()

                    cam.stop_streaming()

        except VimbaCameraError as err:
            self.onResponseFeaturesError.emit(str(err))

I know that this introduces additional overhead in the form of copying the frame before modifying it in-place, but currently this seems to be the simplest way to go about this. Alternatively, you could of course modify the convert_pixel_format method to not change the Frame instance in-place, but instead return a new instance of Frame, where the conversion has been performed. In fact, this change in behaviour is planned for the next major release of VimbaPython.

te-bachi commented 2 years ago

Thank you very much! The Frame and the corresponding buffer is managed by Vimba SDK. I also tried this code before, but it crashed immediately in the Qt slotwhen accessing the QImage's buffer:

    def __frameHandler(self, cam: Camera, frame: Frame):
        if frame.get_status() == FrameStatus.Complete:
            pixelFormat = frame.get_pixel_format()

            if pixelFormat == PixelFormat.Rgb8:
                image = QImage(frame.get_buffer(), frame.get_width(), frame.get_height(), QImage.Format_RGB888)
            elif pixelFormat == PixelFormat.BayerRG8 or pixelFormat == PixelFormat.BayerRG12:
                # convert BayerRG12 to RGB8 with OpenCV
                npFrame = frame.as_numpy_ndarray()
                cvFrame = cv2.cvtColor(npFrame, cv2.COLOR_BAYER_RG2RGBA)
                image = QImage(cvFrame.tobytes(), cvFrame.shape[1], cvFrame.shape[0], QImage.Format_RGBA64)
            else:
                image = QImage(frame.get_buffer(), frame.get_width(), frame.get_height(), QImage.Format_Grayscale8)

            self.onFrameReceived.emit(image)

        cam.queue_frame(frame)

image.copy() doesn't help neither. But with copy.deepcopy(frame), it copies the whole frame with the corresponding buffer.

                # convert BayerRG12 to RGB8 with OpenCV
                frame_cpy = copy.deepcopy(frame)
                npFrame = frame_cpy.as_numpy_ndarray()
                cvFrame = cv2.cvtColor(npFrame, cv2.COLOR_BAYER_RG2RGBA)
                image = QImage(cvFrame.tobytes(), cvFrame.shape[1], cvFrame.shape[0], QImage.Format_RGBA64)

One downside is the Pythons garbage collector, that doesn't free it for a long time: RAM usage resolved

Using convert_pixel_format() or cv2.cvtColor() works finally!

NiklasKroeger-AlliedVision commented 2 years ago

Glad the suggestion worked.

One downside is the Pythons garbage collector, that doesn't free it for a long time

Just as a side note, you can force garbage collection manually using the collect function from the garbage collector interface gc

import gc

gc.collect()  # force garbage collection

This call will block until garbage collection is finished. Keep this in mind if you use it in performance critical sections of your code such as the frame callback.

te-bachi commented 2 years ago

With 18 fps the workaround with copy.deepcopy(frame) doesn't help much. Memory allocation goes from 300MB to 5GB and back very fast and struggles/freezes when the gc is active.

RAM usage 5GB

I'm not very comfortable using the Module ctypes, so I just copy/paste part of it from Frame.convert_pixel_format()

Here is my solution: "Frame.convert_pixel_format() allocates every time a new buffer. For streaming with 18 fps it's too much overhead allocating and freeing memory. Splitting the method in an allocation/copy-part and a pixelformat-conversion-part is more efficient if you only call the copy-part once." https://github.com/te-bachi/VimbaPython/commit/0aad780522ac1c50306933747f6d6b919e8a3e48

And here is my code:

class VimbaThread(QThread):

    onFirstFrameReceived = Signal()
    onFrameReceived = Signal(QImage, ndarray)

    def __init__(self, parent=None):
        super().__init__(parent)
        [...]
        self.__firstFrame = True
        self.frameCopy: Union[Frame, None] = None
        self.frameCopyRaw: Union[Frame, None] = None

    [...]

    def __frameHandler(self, cam: Camera, frame: Frame):
        """This handler is used by the Vimba library for streaming frames.
        It acquires a frame, converts a raw Vimba frame to RGB and sends it to the GUI
        """

        if frame.get_status() == FrameStatus.Complete:
            raw: ndarray = None
            image: QImage = None

            # if it's the first frame that was received...
            if self.__firstFrame:
                self.__firstFrame = False

                # copy two instances of class Frame
                self.frameCopy = frame.copy_as(PixelFormat.Rgb8)
                self.frameCopyRaw = frame.copy_as(PixelFormat.Rgb16)

                # ... send a signal to the GUI
                self.onFirstFrameReceived.emit()

            # convert to RGB8
            frame.convert_to(self.frameCopy)

            # convert to RGB16
            frame.convert_to(self.frameCopyRaw)

            # create QImage image (8-bit)
            image = QImage(self.frameCopy.get_buffer(), self.frameCopy.get_width(), self.frameCopy.get_height(), QImage.Format_RGB888)

            # create numpy image (12-bit)
            raw = self.frameCopyRaw.as_numpy_ndarray()

            # sends it to the GUI thread
            if image is not None:
                self.onFrameReceived.emit(image, raw)

        # consume (reuse) the frame
        cam.queue_frame(frame)

The python process doesn't go over 300MB anymore: RAM usage 300MB

Maybe there is still a race-condition, but for now it's ok for me. Are there plans to introduce these two-part methods in the official VimbaPython repo?

NiklasKroeger-AlliedVision commented 2 years ago

Thanks a lot for the feedback on this! This is a great idea and I will try to integrate this into VimbaPython. As I mentioned, in a future version the convert_pixel_format method will change to return a copy of the frame in the target format (essentially the same behaviour as your copy_as method will become the default behaviour of convert_pixel_format). But I had not though about giving the option to reuse an existing buffer to store the transformation result in to prevent the garbage collector issue. I will definitely look into how we can integrate this in VimbaPython! Thank you for providing a working code example on how you implemented it.

mirco-ackermann commented 2 years ago

I think I ran into the same problem. I changed the convert_pixel_format() to take a optional destination_buffer and write to that if given, leaving the frame unmodified. https://github.com/mirco-ackermann/VimbaPython/blob/823f671a6bd71b5cff66e317b6c1224be2c201df/vimba/frame.py#L759 With this hack I no longer have any garbage collector problems without doing the copy, tested with my own program and with a modified asynchronous_grab.py that is in the same commit. For ease of use it might make sense for to_numpy_array() to take a PixelFormat and do the conversion if required.