Fix #114342: Crash when opening too many movies at once #114381

Richard Antalik · 2023-11-01T22:02:59+01:00

Richard Antalik commented

2023-11-01 22:02:59 +01:00

Adding strips or changing filepaths caused that movie files were loaded,
but memory was only released after new frame was rendered. Since FFmpeg
can take a lot of memory per strip, this can cause crash.

Free memory for each strip immediately after it is not needed after
these operations.

Adding strips or changing filepaths caused that movie files were loaded, but memory was only released after new frame was rendered. Since FFmpeg can take a lot of memory per strip, this can cause crash. Free memory for each strip immediately after it is not needed after these operations.

Richard Antalik added 1 commit 2023-11-01 22:03:13 +01:00

buildbot/vexp-code-patch-coordinator Build done. Details

edf0d55a7b Fix #114342 : Crash when opening too many movies at once

Adding strips or changing filepaths caused that movie files were loaded,
but memory was only released after new frame was rendered. Since FFmpeg
can take a lot of memory per strip, this can cause crash.

Free memory for each strip immediately after it is not needed after
these operations.

Richard Antalik requested review from Sergey Sharybin 2023-11-01 22:03:24 +01:00

Richard Antalik requested review from Bastien Montagne 2023-11-01 22:07:33 +01:00

Bastien Montagne reviewed 2023-11-02 09:50:58 +01:00

Bastien Montagne left a comment

Fix makes sense to me, wondering though if there should not be a check for cfra, to avoid freeing the anim data when the sequence intersects with it?

At some point we'll have to find a way to cache and manage data in anim in a better way, like we already do with the ImBuf - though this is likely way more tricky since it would have to deal with big unknown like the FFMPEG context...

Fix makes sense to me, wondering though if there should not be a check for `cfra`, to avoid freeing the `anim` data when the sequence intersects with it? At some point we'll have to find a way to cache and manage data in `anim` in a better way, like we already do with the `ImBuf` - though this is likely way more tricky since it would have to deal with big unknown like the FFMPEG context...

source/blender/sequencer/intern/strip_add.cc Outdated

						
				@ -627,2 +629,4 @@

				        seq->len = 0;

				      }

				      /* Prevent high memory usage when adding many files at once. */

Bastien Montagne commented

2023-11-02 09:42:59 +01:00

when reloading many files at once

`when reloading many files at once`

Bastien Montagne referenced this pull request

2023-11-02 09:53:01 +01:00

VSE: Crash when setting the filepath for large amount of strips via python #114342

Sergey Sharybin commented

2023-11-02 10:02:02 +01:00

Checking for cfra is not fully enough. You can have multiple videos stacked on top of each others, with OVERRIDE blending. In this case you'd only want to keep the top-most anim in memory.

I don't really think it will be THAT bad to free anim on open/reload, don't think it is that slow. It is also not something you do often. So feels like we'd better first land simple solution, unlock some people in the studio, and look into further improvements as a follow up development.

Checking for `cfra` is not fully enough. You can have multiple videos stacked on top of each others, with OVERRIDE blending. In this case you'd only want to keep the top-most `anim` in memory. I don't really think it will be THAT bad to free anim on open/reload, don't think it is that slow. It is also not something you do often. So feels like we'd better first land simple solution, unlock some people in the studio, and look into further improvements as a follow up development.

👍 1

Bastien Montagne commented

2023-11-02 10:20:51 +01:00

@blender-bot package

Blender Bot commented

2023-11-02 10:20:54 +01:00

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR114381) when ready.

Bastien Montagne commented

2023-11-02 10:22:33 +01:00

Checking for cfra is not fully enough. You can have multiple videos stacked on top of each others, with OVERRIDE blending. In this case you'd only want to keep the top-most anim in memory.

I don't really think it will be THAT bad to free anim on open/reload, don't think it is that slow. It is also not something you do often. So feels like we'd better first land simple solution, unlock some people in the studio, and look into further improvements as a follow up development.

OKI then doing build to confirm it works for the studio TD.

> Checking for `cfra` is not fully enough. You can have multiple videos stacked on top of each others, with OVERRIDE blending. In this case you'd only want to keep the top-most `anim` in memory. > > I don't really think it will be THAT bad to free anim on open/reload, don't think it is that slow. It is also not something you do often. So feels like we'd better first land simple solution, unlock some people in the studio, and look into further improvements as a follow up development. OKI then doing build to confirm it works for the studio TD.

Bastien Montagne commented

2023-11-02 12:09:44 +01:00

@tinynick

👍 1

Bastien Montagne approved these changes 2023-11-02 14:25:09 +01:00

Bastien Montagne left a comment

Fix works fine here at studio, so LGTM.

Sergey Sharybin approved these changes 2023-11-02 14:27:34 +01:00

Sergey Sharybin left a comment

The comment still needs to be updated it seems, but there is no need in extra review iteration.

Richard Antalik added 1 commit 2023-11-03 01:59:14 +01:00

176f58d48b Fix comment

Richard Antalik merged commit 5d765a0d31 into blender-v4.0-release

2023-11-03 02:01:24 +01:00

Richard Antalik referenced this issue from a commit

2023-11-03 02:01:25 +01:00

Fix #114342: Crash when opening too many movies at once

Richard Antalik deleted branch 114342

2023-11-03 02:01:29 +01:00

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix #114342: Crash when opening too many movies at once #114381