| Message ID | 20240305153058.1761020-1-nicolas@ndufresne.ca |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Quoting Nicolas Dufresne (2024-03-05 15:30:55) > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > This set fixes a memory lifetime issue which in the end happens because the > camera manager needs to outlive all of its object, including the > FrameBufferAllocator. This is fixing a crash reported at: > > https://bugs.libcamera.org/show_bug.cgi?id=211 > > Changes in v2: > - Fixed copyright and header comment This series fully passes the CI: - https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1173595 > > Nicolas Dufresne (3): > gstreamer: allocator: Ensure camera manager stay alive > test: gstreamer: Simplify single stream test > test: gstreamer: Test memory lifetime > > src/gstreamer/gstlibcameraallocator.cpp | 16 +++- > .../gstreamer_memory_lifetime_test.cpp | 75 +++++++++++++++++++ > .../gstreamer_single_stream_test.cpp | 27 +++---- > test/gstreamer/meson.build | 4 +- > 4 files changed, 102 insertions(+), 20 deletions(-) > create mode 100644 test/gstreamer/gstreamer_memory_lifetime_test.cpp > > -- > 2.43.2 >
Le jeudi 09 mai 2024 à 15:25 +0100, Kieran Bingham a écrit : > Quoting Nicolas Dufresne (2024-03-05 15:30:55) > > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > This set fixes a memory lifetime issue which in the end happens because the > > camera manager needs to outlive all of its object, including the > > FrameBufferAllocator. This is fixing a crash reported at: > > > > https://bugs.libcamera.org/show_bug.cgi?id=211 > > > > Changes in v2: > > - Fixed copyright and header comment > > This series fully passes the CI: > > - https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1173595 Shall we have a process so this extra effort (even though nice) is not necessary ? The reason I'm saying that is that I already CI tested my changes before sending it, it was preceded by my serie fixing the CI for forks. https://gitlab.freedesktop.org/ndufresne/libcamera/-/tree/gstreamer-lifetime Nicolas p.s. obvious this is a workaround to not using merge request ;-P > > > > > > Nicolas Dufresne (3): > > gstreamer: allocator: Ensure camera manager stay alive > > test: gstreamer: Simplify single stream test > > test: gstreamer: Test memory lifetime > > > > src/gstreamer/gstlibcameraallocator.cpp | 16 +++- > > .../gstreamer_memory_lifetime_test.cpp | 75 +++++++++++++++++++ > > .../gstreamer_single_stream_test.cpp | 27 +++---- > > test/gstreamer/meson.build | 4 +- > > 4 files changed, 102 insertions(+), 20 deletions(-) > > create mode 100644 test/gstreamer/gstreamer_memory_lifetime_test.cpp > > > > -- > > 2.43.2 > >
On Thu, May 09, 2024 at 11:31:01AM -0400, Nicolas Dufresne wrote: > Le jeudi 09 mai 2024 à 15:25 +0100, Kieran Bingham a écrit : > > Quoting Nicolas Dufresne (2024-03-05 15:30:55) > > > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > > > This set fixes a memory lifetime issue which in the end happens because the > > > camera manager needs to outlive all of its object, including the > > > FrameBufferAllocator. This is fixing a crash reported at: > > > > > > https://bugs.libcamera.org/show_bug.cgi?id=211 > > > > > > Changes in v2: > > > - Fixed copyright and header comment > > > > This series fully passes the CI: > > > > - https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1173595 > > Shall we have a process so this extra effort (even though nice) is not necessary > ? The reason I'm saying that is that I already CI tested my changes before > sending it, it was preceded by my serie fixing the CI for forks. I think we should have such a process, yes. We focussed on implementing the test infrastructure first. We'll get to the next step of improving the process at some point. > https://gitlab.freedesktop.org/ndufresne/libcamera/-/tree/gstreamer-lifetime > > Nicolas > > p.s. obvious this is a workaround to not using merge request ;-P > > > > Nicolas Dufresne (3): > > > gstreamer: allocator: Ensure camera manager stay alive > > > test: gstreamer: Simplify single stream test > > > test: gstreamer: Test memory lifetime > > > > > > src/gstreamer/gstlibcameraallocator.cpp | 16 +++- > > > .../gstreamer_memory_lifetime_test.cpp | 75 +++++++++++++++++++ > > > .../gstreamer_single_stream_test.cpp | 27 +++---- > > > test/gstreamer/meson.build | 4 +- > > > 4 files changed, 102 insertions(+), 20 deletions(-) > > > create mode 100644 test/gstreamer/gstreamer_memory_lifetime_test.cpp
Quoting Laurent Pinchart (2024-05-09 16:37:33) > On Thu, May 09, 2024 at 11:31:01AM -0400, Nicolas Dufresne wrote: > > Le jeudi 09 mai 2024 à 15:25 +0100, Kieran Bingham a écrit : > > > Quoting Nicolas Dufresne (2024-03-05 15:30:55) > > > > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > > > > > This set fixes a memory lifetime issue which in the end happens because the > > > > camera manager needs to outlive all of its object, including the > > > > FrameBufferAllocator. This is fixing a crash reported at: > > > > > > > > https://bugs.libcamera.org/show_bug.cgi?id=211 > > > > > > > > Changes in v2: > > > > - Fixed copyright and header comment > > > > > > This series fully passes the CI: > > > > > > - https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1173595 > > > > Shall we have a process so this extra effort (even though nice) is not necessary > > ? The reason I'm saying that is that I already CI tested my changes before > > sending it, it was preceded by my serie fixing the CI for forks. > > I think we should have such a process, yes. We focussed on implementing > the test infrastructure first. We'll get to the next step of improving > the process at some point. > > > https://gitlab.freedesktop.org/ndufresne/libcamera/-/tree/gstreamer-lifetime You could reference tests you've run in the cover letter, or - what I expect is that we should at a minimum have some bot scouring patchwork and running the tests, and replying with the results to the first mail of the series. I can do 'most' of that - but I haven't yet managed to extract the appropriate msg-id from patchwork to know where to reply from a script. -- Kieran > > > > Nicolas > > > > p.s. obvious this is a workaround to not using merge request ;-P > > > > > > Nicolas Dufresne (3): > > > > gstreamer: allocator: Ensure camera manager stay alive > > > > test: gstreamer: Simplify single stream test > > > > test: gstreamer: Test memory lifetime > > > > > > > > src/gstreamer/gstlibcameraallocator.cpp | 16 +++- > > > > .../gstreamer_memory_lifetime_test.cpp | 75 +++++++++++++++++++ > > > > .../gstreamer_single_stream_test.cpp | 27 +++---- > > > > test/gstreamer/meson.build | 4 +- > > > > 4 files changed, 102 insertions(+), 20 deletions(-) > > > > create mode 100644 test/gstreamer/gstreamer_memory_lifetime_test.cpp > > -- > Regards, > > Laurent Pinchart
From: Nicolas Dufresne <nicolas.dufresne@collabora.com> This set fixes a memory lifetime issue which in the end happens because the camera manager needs to outlive all of its object, including the FrameBufferAllocator. This is fixing a crash reported at: https://bugs.libcamera.org/show_bug.cgi?id=211 Changes in v2: - Fixed copyright and header comment Nicolas Dufresne (3): gstreamer: allocator: Ensure camera manager stay alive test: gstreamer: Simplify single stream test test: gstreamer: Test memory lifetime src/gstreamer/gstlibcameraallocator.cpp | 16 +++- .../gstreamer_memory_lifetime_test.cpp | 75 +++++++++++++++++++ .../gstreamer_single_stream_test.cpp | 27 +++---- test/gstreamer/meson.build | 4 +- 4 files changed, 102 insertions(+), 20 deletions(-) create mode 100644 test/gstreamer/gstreamer_memory_lifetime_test.cpp