fix(gstreamer): prevent memory leak when updating video frames#9768
fix(gstreamer): prevent memory leak when updating video frames#9768mn89h wants to merge 3 commits intolvgl:masterfrom
Conversation
|
Hi 👋, thank you for your PR! We've run benchmarks in an emulated environment. Here are the results: ARM Emulated 32b - lv_conf_perf32b
Detailed Results Per Scene
ARM Emulated 64b - lv_conf_perf64b
Detailed Results Per Scene
Disclaimer: These benchmarks were run in an emulated environment using QEMU with instruction counting mode. 🤖 This comment was automatically generated by a bot. |
Reuse a persistent image descriptor and pixel buffer instead of creating a new image source each frame. This prevents repeated texture allocations (notably on OpenGL).
5a1af6b to
a70d132
Compare
AndreCostaaa
left a comment
There was a problem hiding this comment.
Hey Malte, what is the motivation for this change?
|
Hi Andre :) I had the problem of memory leaks with GStreamer in conjunction with OpenGLES. It fixes two things: first, it stops the GLES backend from constantly allocating and leaking new textures in VRAM. Since I'm now reusing a persistent pixel buffer and only calling lv_image_set_src once, the renderer can just update the existing texture in-place instead of creating a new one X times a second. |
- check for current state and return if no update needed - wait for pipeline transitioning before start freeing memory - drop image cache before freeing pixel buffer and on resolution change
|
I have discovered some more things, especially in caps handling after first frame, as well as some memory related stuff. In my tests it seems stable now, especially with many state changes after longer periods, where i would repeatedly get crashes. |
f449e4e to
ac20d4b
Compare
- update caps not only on first_frame, but also on changes (e.g. resolution) - added null check when pad is added
ac20d4b to
47fb81f
Compare
The pipeline is configured to send a frame every LV_DEF_REFR_PERIOD and internally we're fetching a sample every LV_DEF_REFR_PERIOD / 5 ms Is the lag caused by a huge render time/flush time ? |
AndreCostaaa
left a comment
There was a problem hiding this comment.
I'm of the opinion we need to figure something else out for the frame updating part of the PR
What you're going for is the original implementation of gstreamer but it relies on copying each frame which is slow and not necessary, it was updated afterwards to avoid the memcpy. Specially if the problem is with draw opengles, I believe we need to figure something else with it.
Also note that for LVGL v9.5 I recommend using DRAW_NANOVG instead of DRAW_OPENGLES
I'll take a look at it as well
I like the other changes tho, can we split these into different PRs?
|
|
||
| if(streamer->pixel_buffer) { | ||
| lv_image_cache_drop(&streamer->frame); | ||
| free(streamer->pixel_buffer); |
There was a problem hiding this comment.
| free(streamer->pixel_buffer); | |
| lv_free(streamer->pixel_buffer); |
Or better yet, what about using realloc instead of free + malloc?
| } | ||
|
|
||
| /* Copy new pixels */ | ||
| memcpy(streamer->pixel_buffer, map.data, required_size); |
There was a problem hiding this comment.
Suggested change
memcpy(streamer->pixel_buffer, map.data, required_size);
lv_memcpy(streamer->pixel_buffer, map.data, required_size);
This is a blocker for the PR, memcpying each frame is something we want to avoid, it will make gstreamer slower for everyone that is not using draw_opengles
|
|
||
| /* Copy new pixels */ | ||
| memcpy(streamer->pixel_buffer, map.data, required_size); | ||
| if(!streamer->image_src_set) { |
There was a problem hiding this comment.
I guess we can get rid of this image_src_set attribute and instead just move lv_image_set_src inside the
streamer->pixel_buffer == NULL || streamer->pixel_buffer_size != required_size condition
| if(streamer->pixel_buffer) { | ||
| /* Drop the image from cache before freeing the buffer */ | ||
| lv_image_cache_drop(&streamer->frame); | ||
| free(streamer->pixel_buffer); |
There was a problem hiding this comment.
| free(streamer->pixel_buffer); | |
| lv_free(streamer->pixel_buffer); |
Reuse a persistent image descriptor and pixel buffer instead of creating a new image source each frame. This prevents repeated texture allocations (notably on OpenGL).