From 64198971eaa27043e60d5164710edcd239aa8766 Mon Sep 17 00:00:00 2001
From: Aras Pranckevicius <aras@nesnausk.org>
Date: Fri, 15 Nov 2024 11:51:49 +0100
Subject: [PATCH] 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 30dbb7820d6 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
---
 source/blender/blenkernel/intern/scene.cc    | 20 ++++++++++++++------
 source/blender/makesdna/DNA_sequence_types.h |  6 ++++++
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/source/blender/blenkernel/intern/scene.cc b/source/blender/blenkernel/intern/scene.cc
index f92b3cf7a07..69e87fc2495 100644
--- a/source/blender/blenkernel/intern/scene.cc
+++ b/source/blender/blenkernel/intern/scene.cc
@@ -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) {
diff --git a/source/blender/makesdna/DNA_sequence_types.h b/source/blender/makesdna/DNA_sequence_types.h
index ac6058a96fd..ca955f60368 100644
--- a/source/blender/makesdna/DNA_sequence_types.h
+++ b/source/blender/makesdna/DNA_sequence_types.h
@@ -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];