Fix: Reading some old blend files fires assert in Sequencer data read

wdas_cloud.blend file on 4.3+ raises an assert, due to reading some
VSE data:

  readfile.cc:4798, blo_verify_data_address(), at
  'MEM_allocN_len(new_address) >= expected_size' Corrupt .blend
  file, unexpected data size.

This is caused by 30dbb7820d which removed completely unused
data from Sequence struct. But it turns out, Sequence serialization
inside scene_blend_read_data is "somewhat strange", to put it mildly,
and silently assumes that struct offset of seqbase and channels
will never change.

For now, restore the previously expected struct member offset,
ensure it stays like that via static_assert, and add notes on
how someone should fix this in a better way.

Pull Request: https://projects.blender.org/blender/blender/pulls/130296
This commit is contained in:
Aras Pranckevicius
2024-11-15 11:51:49 +01:00
committed by Aras Pranckevicius
parent d1171635ff
commit 64198971ea
2 changed files with 20 additions and 6 deletions

View File

@@ -1327,14 +1327,22 @@ static void scene_blend_read_data(BlendDataReader *reader, ID *id)
/* link metastack, slight abuse of structs here,
* have to restore pointer to internal part in struct */
{
Sequence temp;
void *seqbase_poin;
void *channels_poin;
intptr_t seqbase_offset;
intptr_t channels_offset;
seqbase_offset = intptr_t(&(temp).seqbase) - intptr_t(&temp);
channels_offset = intptr_t(&(temp).channels) - intptr_t(&temp);
/* This whole thing with seqbasep offsets is really not good
* and prevents changes to the Sequence struct. A more correct approach
* would be to calculate offset using sDNA from the file (NOT from the
* current Blender). Even better would be having some sort of dedicated
* map of seqbase pointers to avoid this offset magic. */
constexpr intptr_t seqbase_offset = offsetof(Sequence, seqbase);
constexpr intptr_t channels_offset = offsetof(Sequence, channels);
#if ARCH_CPU_64_BITS
static_assert(seqbase_offset == 264, "Sequence seqbase member offset cannot be changed");
static_assert(channels_offset == 280, "Sequence channels member offset cannot be changed");
#else
static_assert(seqbase_offset == 204, "Sequence seqbase member offset cannot be changed");
static_assert(channels_offset == 212, "Sequence channels member offset cannot be changed");
#endif
/* seqbase root pointer */
if (ed->seqbasep == old_seqbasep) {

View File

@@ -231,6 +231,11 @@ typedef struct Sequence {
/* pointers for effects: */
struct Sequence *seq1, *seq2;
/* This strange padding is needed due to how seqbasep deserialization is
* done right now in #scene_blend_read_data. */
void *_pad7;
int _pad8[2];
/** List of strips for meta-strips. */
ListBase seqbase;
ListBase channels; /* SeqTimelineChannel */
@@ -289,6 +294,7 @@ typedef struct Sequence {
float speed_factor;
struct SeqRetimingKey *retiming_keys;
void *_pad5;
int retiming_keys_num;
char _pad6[4];