femalemonkeyman commented 1 year ago

I' just noticing that the memory taken up by both my own and the test application on linux (I have not tested other platforms) is never clearing. It is fine to begin with but eventually does get out of hand. I looked a bit and didn't really see an issue with flutter itself pertaining to this so i was just wondering if there was any insight.

alexmercerind commented 1 year ago

You can see the attached video in README showing test application running on Windows. The memory is free'd when going back from the video render screen.

Flutter shell or Dart VM for GNU/Linux is underperforming. There are many other issues as well. For instance: #64.

srix55 commented 1 year ago

Not able to figure this one out. Memory leak still happening. What I tried :-

As some suggested in a libmpv issue, I upgraded libmpv to master (0.35.1+) using mpv-build
Checked on any errors with mpv logs - no errors
Event mpv-end-file(called when file unloads) is getting called - no errors here
Checked if loadfile is getting called twice - no. It's getting called only once per load
Disabled audio tracks with aid -> no
In dispose of player
- property idle -> no
- property keep-open -> no
- commands stop, playlist-clear, playlist-play-index none & then quit
- mpv_destroy / mpv_terminate_destroy - terminates the core player / the app respectively

alexmercerind commented 1 year ago

I have experimented with it a lot before. Think yourself, if same code is working correctly on Windows & macOS (memory gets free'd as intended). Certainly, something is wrong within the Flutter embedder or Dart VM for GNU/Linux. At least how it treats memory or garbage collects it. There are no memory leaks in my code.

IIRC even if you don't create any VideoController & just Player, memory will still leak on GNU/Linux. I can't do anything at least in the short term for GNU/Linux. Best recommendation I can give is to keep a pool of Player & VideoControllers.

srix55 commented 1 year ago

It's happening in Windows too unfortunately. Here's a demo that loops on a test video - but initializes & destroys player every time - demo

It surely doesn't crash as fast as in linux. Takes a bit longer to pile up than linux. For me, win 10 - 2gb gpu, it took about 7 mins to go from 120 mb to 500 mb.

alexmercerind commented 1 year ago

Let me correct it:

It never crashes like GNU/Linux. Takes far-far longer to grow as compared to GNU/Linux but never exceeds a limit in idle state.

https://user-images.githubusercontent.com/28951144/229049817-7ad75df2-eb76-4046-a9bf-22ce5c1a4634.mp4

It's more as to how Dart VM treats the memory, how OS schedules things etc. You can see that memory is freed automatically sometime afterwards, it can be: Dart VM itself, libmpv internals e.g. demuxer cache, FFmpeg internals, Windows scheduler etc. If you leave it suspended for more time, the memory drops further down.

Note that mpv actually occupies space for cache in memory which is necessary for smooth playback of any kind. You can adjust it with --demuxer-max-bytes. I won't really recommend it. Since... it's not really a problem, the memory usage is fine. Improvements can definitely be made over time.

But yeah... GNU/Linux situation is serious.

The pool based solution should still be helpful. Who knows... what if Google Chrome also recycles same instances once created. Realistically, you are never going to have more than 5 at once.

alexmercerind commented 1 year ago

I have a good news & a bad news.

Good: The memory clean-up has been further improved for all platforms. Bad: GNU/Linux still doesn't free any memory. I still can't identify what's causing it.

srix55 commented 1 year ago

I have spent about 20-25 hours on this in the past months... and am still clueless. Tried so many combinations of flags. Not sure if it's mpv core or if it's gnu or if something else. Definitely nothing to do with media_kit code... i think, not sure if something weird happens over texture / angle. Will look into it again when I get the time. Leaving videos running in the background for long durations is critical for my case (digital signage player).

PurshuRana commented 11 months ago

Unfortunately this happened in IOS also, here is the log i got when app crashed thread #10, name = 'io.flutter.1.ui', stop reason = EXC_RESOURCE (RESOURCE_TYPE_MEMORY: high watermark memory limit exceeded) (limit=1450 MB) frame #0: 0x00000001232f0d14 -> 0x1232f0d14: stur d0, [x0, #0x7] 0x1232f0d18: stp x0, x3, [x15] 0x1232f0d1c: mov x0, x4 0x1232f0d20: ldr x4, [x27, #0x10]

alexmercerind commented 11 months ago

How sure are you that's not a bug in your code? Can't really help against vaguely said statements.

We require clean minimal code that can be used to reproduce the issue. In either case, using existing Player instance e.g. in a singleton is preferred instead of creating new one each time. Either way, our test application is used for most debugging.

PurshuRana commented 11 months ago

Im disposing the player properly before assing the new player instance.

ndusart commented 10 months ago

Hello.

For people experiencing this on Linux (@femalemonkeyman , @srix55 ) Can I ask you which version of glibc you are running on ? Latest version 2.37 introduced some new behavior that prevent some memory reallocation patterns to actually reclaim unused space: https://sourceware.org/bugzilla/show_bug.cgi?id=30579

This is fixed for next release and for 2.37 but most distributions does not ship this patch yet.

Using glibc 2.37, I have a huge memory leak when lauching the multiple reader test case. I go to 30G in about 2minutes. This memory is never reclaimed when going back to menu and waiting.

Tried the test example with mimalloc and didn't experience the leak. Memory raise a bit while playing videos but reduce when going back to the menu.

srix55 commented 10 months ago

@ndusart Mine is ldd (Ubuntu GLIBC 2.35-0ubuntu3.1) 2.35 In another issue, someone said that it's taking too long for mpv to get destroyed and that's causing a buildup to a leak - #295 and it seems to be fixed now. I am yet to test the latest version.

alexmercerind commented 10 months ago

Hi @ndusart,

Your comment & the bug-report you linked is quite interesting.

The memory issue is solely present on GNU/Linux & seemingly memory never gets freed. You can see the video I attached above (Windows), where memory gets freed as intended. It has been further improved after the latest updates, it's been 5 months since then.

The project will be greatly helped with any workaround or fixes. We currently bundle package:ffi v1.2.1 within package:media_kit.

I'm really curious to know how you tested our example application with mimalloc (updating native video rendering code? updating package:ffi calloc?). And, if the improvements are geniune, consider raising a PR.

I have no hate for GNU/Linux, but there are many issues, it's like nothing is stable & the quality is also not optimal. Few issues other than this still remain:

As it seems, GDK/GL can be even slower than software rendering: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/6042
After recent Flutter updates, the subsequent Texture widgets don't work with GL (& our software code path starts working) i.e. first Texture widget will work correctly (hardware accelerated) & rest will fail to initialize GL context.

femalemonkeyman commented 10 months ago

@ndusart Arch glibc is at 2.37-3 which is where I am

alexmercerind commented 10 months ago

I just tried out mimalloc with our test application & the improvement is real. Now memory gets released as intended on GNU/Linux, some improvement is still needed (but very-very clearly way better than before).

ndusart commented 10 months ago

I'm really curious to know how you tested our example application with mimalloc (updating native video rendering code? updating package:ffi calloc?). And, if the improvements are geniune, consider raising a PR.

You figured that out already it seemed. libc memory allocator can be overriden thanks to LD_PRELOAD env var.

@femalemonkeyman Yes I am on arch too and the patch has not been landed yet. This can explain part of the issue.

@alexmercerind I hope this is really the issue here, I thought of testing it because I had similar issues in some of projects at work. But @srix55 seem to have some memory allocation issue on older glibc too. Maybe the glibc 2.37 behavior also added another memory issue on top of another one.

But still, the stress test is raising indefinitely in memory for whatever memory allocator I use. I do not know if it is normal (that is still a stress test after all ^^).

I tried to take a look at the code of media_kit but I would take me too much effort to understand all the architecture. Sorry if that may sound a bit pessimistic, I try to avoid to be either optimistic or pessimistic about the fact that this bug is due entirely to glibc 2.37 but it is likely to play some role in it.

alexmercerind commented 10 months ago

Hi @ndusart,

You figured that out already it seemed. libc memory allocator can be overriden thanks to LD_PRELOAD env var.

Yeah! I used it later (made comment early in excitement ^^). I have wasted my weeks trying to fix this GNU/Linux memory leak.

But still, the stress test is raising indefinitely in memory for whatever memory allocator I use.

No, it's not normal. It doesn't happen on other platforms (Windows, Android, macOS & iOS).

For the note, I'm still going to blame glibc & it is only it's fault. Same implementation works 100% correctly & does not leak on other platforms.

I had this memory issue on GNU/Linux long before I implemented video rendering (FYI: package:media_kit was created to power Harmonoid ~ a music player; no video rendering. You can still see me getting bashed in the issues for performance on GNU/Linux). The memory used to leak even when there was no video embedding. I honestly didn't know that it's this easy to switch allocators ^^ (& if it was even possible).

alexmercerind commented 10 months ago

Now, for the good-news:

But still, the stress test is raising indefinitely in memory for whatever memory allocator I use.

So, actual leak was fixed by switching to mimalloc.

First of all, I don't have experience in GTK/GObject etc. I've mostly worked with Win32 & standard C++. In current implementation, I followed the recommended "GObject practices" as good as I could. Still... I'm not very familiar with it.

It turns out I was leaking a new instance of GThread* upon every render in software rendering! I wanted to create a detached thread, somewhat like std::thread::detach which automatically frees itself once it's execution is done.

I fixed this in: https://github.com/media-kit/media-kit/pull/319 :tada:

Now a good question may be, why was memory growing in stress-test if it's only in software rendering, aren't we using hardware rendering by default? Like I said in the comment above:

After recent Flutter updates, the subsequent Texture widgets don't work with GL

When multiple Videos are mounted, only first one uses H/W & rest fallback to S/W due to issue introduced by recent Flutter version. This didn't happen before & I will file an issue at flutter/flutter.

alexmercerind commented 10 months ago

Now it's not leaking at all.

srix55 commented 10 months ago

Screencast from 04-08-23 08:50:18 PM IST.webm

You are right. This seems to fix the leak :) Thanks @alexmercerind & @ndusart . I was testing it the whole day. (Although my application still leaks - i think it's because of the pre-fix myriad optimizations that are getting in the way). I did an isolated test with 6 concurrent players disposing & playing short videos at random. Doesn't seem to leak memory. :balloon: :partying_face: :balloon:

alexmercerind commented 10 months ago

@srix55 the CPU usage should be 0.0% ideally.

329 will reduce the CPU usage by large margin (like in the earlier versions). Now multiple videos use hardware decoding correctly & don't fallback to software decoding.

That 0.0% only works on NVIDIA devices only for now (through NVDEC). VAAPI unfortunately will only work with -copy on GTK 3.0.

On my entry-level Ryzen 3 2200 U with integrated Radeon Vega 3, I'm able to play around 20 HD videos at once without any lag. Multiple 4K/8K 60 FPS also work as intended. The performance should be higher on capable devices.

alexmercerind commented 10 months ago

A final note for the future visitors:

Memory leaks on GNU/Linux have been resolved & it is recommended to use mimalloc in your project:

Refer to "Utilize mimalloc" section in the README.

You just need to add one line in your linux/CMakeLists.txt:

target_link_libraries(${BINARY_NAME} PRIVATE ${MIMALLOC_LIB})

I tried to eliminate this requirement of manual modification from developer's end, but it's not possible.

alexmercerind commented 10 months ago

It'd be good idea to let glibc's developers know what's wrong.

media-kit / media-kit

[Bug Report][GNU/Linux] Memory usage not freeing with dispose #68

329 will reduce the CPU usage by large margin (like in the earlier versions). Now multiple videos use hardware decoding correctly & don't fallback to software decoding.