mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
28.71k stars 2.93k forks source link

common/playlist: don't allocate duplicated playlist_path #15323

Closed kasper93 closed 1 week ago

kasper93 commented 1 week ago

If the playlist is loaded directly from a protocol like memory://, the playlist_path represents the entire playlist. In cases where we have a large playlist, this results in the entire playlist being duplicated for each item. For example, if the input size is 300 kB with 10k items, we end up using 3 GB of memory just to store the playlist_path strings.

github-actions[bot] commented 1 week ago

Download the artifacts for this pull request:

Windows * [mpv-i686-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/2197494655.zip) * [mpv-x86_64-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/2197500084.zip) * [mpv-x86_64-windows-msvc](https://nightly.link/mpv-player/mpv/actions/artifacts/2197510811.zip)
macOS * [mpv-macos-13-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/2197491009.zip) * [mpv-macos-14-arm](https://nightly.link/mpv-player/mpv/actions/artifacts/2197490154.zip) * [mpv-macos-15-arm](https://nightly.link/mpv-player/mpv/actions/artifacts/2197493504.zip)
zsugabubus commented 1 week ago

Sorry if I'm stating the obvious, I just would like to note that long memory:// will still remain very inefficient (=unusable) because playlist property keeps serializing that huge playlist_path for each entry.

kasper93 commented 1 week ago

Indeed, it is quite problematic, might fix that in the future. For now at least it will not OOM like crazy...

llyyr commented 1 week ago

Open a ticket so it's not memory-holed