Maniphest T41086

VSE separate images increases file size abnormally
Closed, ResolvedPublic
Actions

Description

In official Blender 2.71, by spliting a strip with a sequence of 722 images with Separate images function bpy.ops.sequencer.images_separate(length=x)
the .blend file jumps from 600 Kb to something near 132 Mb, without any packed file.

It seems a bit exagerated compared to the same steps done in 2.70a, where it increases from 570 kb to 4.4 Mb, much more reasonable.

System Information
Scientific Linux (CentOS like)
Linux lajeado 2.6.32-431.20.3.el6.x86_64 #1 SMP Thu Jun 19 14:01:59 CDT 2014 x86_64 x86_64 x86_64 GNU/Linux
Graphics card: 0f:00.0 VGA compatible controller: NVIDIA Corporation GT200GL [Quadro FX 3800] (rev a1)

Blender Version
Broken: 2.71 hash: 9337574
Worked: 2.70a

Short description of error

Exact steps for others to reproduce the error
Based on a (as simple as possible) attached .blend file with minimum amount of steps

Details

Commits
rB2e436173aae1: Fix T41086: VSE separate images increases file size abnormally.
Type
Bug
Bastien Montagne (mont29) triaged this task as Normal priority.Jul 17 2014, 8:24 PM
Bastien Montagne (mont29) added a subscriber: Bastien Montagne (mont29).

I cannot reproduce that here (linux64), neither with official 2.71 nor current trunk, I’m always getting same results (6.8 Mo for about 150 images - which I found already rather high, tbh!).

Sergey Sharybin (sergey) added a comment.Jul 18 2014, 10:46 AM

Please always attahc all files needed to reproduce the issue.

I can only think you've got really long sequence and separating it for a single frame, which leads to loads to individual strips. This would for sure bump file size dramatically.

Bastien Montagne (mont29) added a comment.Jul 18 2014, 4:27 PM

@Sergey Sharybin (sergey) would you consider bump from 500k to 6.8M (from 1 strip to 150) the expected amount of increase mem? That would make about 40k per strip, sounds a bit high to me, tbh…

Also, I can have a look at that myself, if you want (just assign it to me in this case).

Sergey Sharybin (sergey) assigned this task to Bastien Montagne (mont29).Jul 21 2014, 5:40 PM

40K indeed looks a bit suspicious. Definitely worth checking. Please go ahead :)

Sergey Sharybin (sergey) added a comment.Jul 21 2014, 5:41 PM

Also guess "uncofirmed" should be removed now?

Bastien Montagne (mont29) added a comment.Jul 21 2014, 10:34 PM

Found the culprit - we copy everything from the old sequence into each new ones… including the stripdata, which for image sequences is an array with one item per image!

Following patch fixes this in the simplest way possible, not sure it would be considered good enough for master, Sergey?

P93 T41086
1​diff --git a/source/blender/editors/space_sequencer/sequencer_edit.c b/source/blender/editors/space_sequencer/sequencer_edit.c
2​index 973d6a9..dcf13db 100644
3​--- a/source/blender/editors/space_sequencer/sequencer_edit.c
4​+++ b/source/blender/editors/space_sequencer/sequencer_edit.c
5​@@ -1919,9 +1919,12 @@ static int sequencer_separate_images_exec(bContext *C, wmOperator *op)
6​ strip_new = seq_new->strip;
7​ strip_new->us = 1;
8​
9​- /* new stripdata */
10​- se_new = strip_new->stripdata;
11​+ /* new stripdata (only one element now!) */
12​+ /* Note this assume all elements (images) have the same dimension, since we only copy the name here. */
13​+ se_new = MEM_reallocN(strip_new->stripdata, sizeof(*se_new));
14​ BLI_strncpy(se_new->name, se->name, sizeof(se_new->name));
15​+ strip_new->stripdata = se_new;
16​+
17​ BKE_sequence_calc(scene, seq_new);
18​
19​ if (step > 1) {