struct Rav1dFrameContext_task_thread: Make Rav1dTask lists indexed

Rav1dTasks are stored in two arrays and linked
into a linked list by pointers. This change
refactors the lists into a `Rav1dTasks` structure
that links tasks by index instead of pointer.

We still access the Rav1dTasks structure with
unsafe interior mutability, because we would have
borrowing errors between the Rav1dFrameData
reference lifetime and the lifetime of borrows out
of the Rav1dTasks structure. This will be fixed
soon when we split `Rav1dFrameData` into an
immutable, interiorly mutable portion and a mutex
protected mutable portion.

src/internal.rs

@@ -73,9 +73,16 @@ use crate::src::refmvs::refmvs_tile;
 use crate::src::refmvs::Rav1dRefmvsDSPContext;
 use atomig::Atomic;
 use libc::ptrdiff_t;
+use std::cell::UnsafeCell;
+use std::cmp;
 use std::ffi::c_int;
 use std::ffi::c_uint;
 use std::mem;
+use std::ops::Add;
+use std::ops::AddAssign;
+use std::ops::Index;
+use std::ops::IndexMut;
+use std::ops::Sub;
 use std::ptr;
 use std::sync::atomic::AtomicBool;
 use std::sync::atomic::AtomicI32;
@@ -127,6 +134,12 @@ pub enum TaskType {
     FgApply = 12,
 }
 
+impl Default for TaskType {
+    fn default() -> Self {
+        Self::Init
+    }
+}
+
 #[repr(C)]
 pub(crate) struct Rav1dContext_frame_thread {
     pub out_delayed: Box<[Rav1dThreadPicture]>,
@@ -294,7 +307,7 @@ unsafe impl Send for Rav1dContext {}
 // TODO(SJC): Remove when Rav1dContext is thread-safe
 unsafe impl Sync for Rav1dContext {}
 
-#[derive(Clone)]
+#[derive(Clone, Default)]
 #[repr(C)]
 pub struct Rav1dTask {
     // frame thread id
@@ -309,7 +322,7 @@ pub struct Rav1dTask {
     pub deblock_progress: c_int,
     pub deps_skip: c_int,
     // only used in task queue
-    pub next: *mut Rav1dTask,
+    pub next: Option<Rav1dTaskIndex>,
 }
 
 #[repr(C)]
@@ -474,17 +487,120 @@ pub struct Rav1dFrameContext_lf {
     pub restore_planes: c_int, // enum LrRestorePlanes
 }
 
+#[derive(Default)]
 #[repr(C)]
 pub struct Rav1dFrameContext_task_thread_pending_tasks {
-    pub head: *mut Rav1dTask,
-    pub tail: *mut Rav1dTask,
+    pub head: Option<Rav1dTaskIndex>,
+    pub tail: Option<Rav1dTaskIndex>,
 }
 
-impl Default for Rav1dFrameContext_task_thread_pending_tasks {
-    fn default() -> Self {
-        Self {
-            head: ptr::null_mut(),
-            tail: ptr::null_mut(),
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+pub enum Rav1dTaskIndex {
+    Task(usize),
+    TileTask(usize),
+    Init,
+}
+
+impl Rav1dTaskIndex {
+    pub fn raw_index(self) -> Option<usize> {
+        match self {
+            Self::Task(i) => Some(i),
+            Self::TileTask(i) => Some(i),
+            Self::Init => None,
+        }
+    }
+}
+
+impl Sub for Rav1dTaskIndex {
+    type Output = Self;
+
+    fn sub(self, rhs: Self) -> Self::Output {
+        match (self, rhs) {
+            (Self::Task(x), Self::Task(y)) => Self::Task(x - y),
+            (Self::TileTask(x), Self::TileTask(y)) => Self::TileTask(x - y),
+            _ => panic!("Cannot subtract {rhs:?} from {self:?}"),
+        }
+    }
+}
+
+impl PartialOrd for Rav1dTaskIndex {
+    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
+        match (self, other) {
+            (Self::Task(x), Self::Task(y)) => x.partial_cmp(y),
+            (Self::TileTask(x), Self::TileTask(y)) => x.partial_cmp(y),
+            _ => None,
+        }
+    }
+}
+
+impl Add<usize> for Rav1dTaskIndex {
+    type Output = Self;
+
+    fn add(self, rhs: usize) -> Self::Output {
+        match self {
+            Self::Task(i) => Self::Task(i + rhs),
+            Self::TileTask(i) => Self::TileTask(i + rhs),
+            Self::Init => panic!("Cannot add to the init task"),
+        }
+    }
+}
+
+impl AddAssign<usize> for Rav1dTaskIndex {
+    fn add_assign(&mut self, rhs: usize) {
+        *self = *self + rhs;
+    }
+}
+
+#[derive(Default)]
+pub struct Rav1dTasks {
+    tasks: Vec<Rav1dTask>,
+    tile_tasks_vec: Vec<Rav1dTask>,
+    init_task: Rav1dTask,
+    pub tile_tasks: [Option<Rav1dTaskIndex>; 2],
+    pub head: Option<Rav1dTaskIndex>,
+    pub tail: Option<Rav1dTaskIndex>,
+    // Points to the task directly before the cur pointer in the queue.
+    // This cur pointer is theoretical here, we actually keep track of the
+    // "prev_t" variable. This is needed to not loose the tasks in
+    // [head;cur-1] when picking one for execution.
+    pub cur_prev: Option<Rav1dTaskIndex>,
+}
+
+impl Rav1dTasks {
+    pub fn grow_tasks(&mut self, new_len: usize) {
+        if new_len > self.tasks.len() {
+            self.tasks.clear();
+            self.tasks.resize_with(new_len, Default::default);
+        }
+    }
+
+    pub fn grow_tile_tasks(&mut self, new_len: usize) {
+        if new_len > self.tile_tasks_vec.len() {
+            self.tile_tasks_vec.clear();
+            self.tile_tasks_vec.resize_with(new_len, Default::default);
+            self.tile_tasks[0] = Some(Rav1dTaskIndex::TileTask(0));
+        }
+    }
+}
+
+impl Index<Rav1dTaskIndex> for Rav1dTasks {
+    type Output = Rav1dTask;
+
+    fn index(&self, index: Rav1dTaskIndex) -> &Self::Output {
+        match index {
+            Rav1dTaskIndex::Task(index) => &self.tasks[index],
+            Rav1dTaskIndex::TileTask(index) => &self.tile_tasks_vec[index],
+            Rav1dTaskIndex::Init => &self.init_task,
+        }
+    }
+}
+
+impl IndexMut<Rav1dTaskIndex> for Rav1dTasks {
+    fn index_mut(&mut self, index: Rav1dTaskIndex) -> &mut Self::Output {
+        match index {
+            Rav1dTaskIndex::Task(index) => &mut self.tasks[index],
+            Rav1dTaskIndex::TileTask(index) => &mut self.tile_tasks_vec[index],
+            Rav1dTaskIndex::Init => &mut self.init_task,
         }
     }
 }
@@ -494,29 +610,24 @@ pub(crate) struct Rav1dFrameContext_task_thread {
     pub lock: Mutex<()>,
     pub cond: Condvar,
     pub ttd: Arc<TaskThreadData>,
-    pub tasks: *mut Rav1dTask,
-    pub tile_tasks: [*mut Rav1dTask; 2],
-    pub init_task: Rav1dTask,
-    pub num_tasks: c_int,
-    pub num_tile_tasks: c_int,
+    pub tasks: UnsafeCell<Rav1dTasks>,
     pub init_done: AtomicI32,
     pub done: [AtomicI32; 2],
     pub retval: Rav1dResult,
     pub update_set: bool, // whether we need to update CDF reference
     pub error: AtomicI32,
     pub task_counter: AtomicI32,
-    pub task_head: *mut Rav1dTask,
-    pub task_tail: *mut Rav1dTask,
-    // Points to the task directly before the cur pointer in the queue.
-    // This cur pointer is theoretical here, we actually keep track of the
-    // "prev_t" variable. This is needed to not loose the tasks in
-    // [head;cur-1] when picking one for execution.
-    pub task_cur_prev: *mut Rav1dTask,
     // async task insertion
     pub pending_tasks_merge: AtomicI32,
     pub pending_tasks: Mutex<Rav1dFrameContext_task_thread_pending_tasks>,
 }
 
+impl Rav1dFrameContext_task_thread {
+    pub unsafe fn tasks(&self) -> *mut Rav1dTasks {
+        self.tasks.get()
+    }
+}
+
 // threading (refer to tc[] for per-thread things)
 #[repr(C)]
 pub struct FrameTileThreadData {

src/lib.rs

@@ -33,7 +33,6 @@ use crate::src::internal::Rav1dContext;
 use crate::src::internal::Rav1dContextTaskThread;
 use crate::src::internal::Rav1dContextTaskType;
 use crate::src::internal::Rav1dFrameData;
-use crate::src::internal::Rav1dTask;
 use crate::src::internal::Rav1dTaskContext;
 use crate::src::internal::Rav1dTaskContext_task_thread;
 use crate::src::internal::TaskThreadData;
@@ -70,6 +69,7 @@ use libc::pthread_attr_destroy;
 use libc::pthread_attr_init;
 use libc::pthread_attr_setstacksize;
 use libc::pthread_attr_t;
+use std::cell::UnsafeCell;
 use std::cmp;
 use std::ffi::c_char;
 use std::ffi::c_int;
@@ -337,6 +337,7 @@ pub(crate) unsafe fn rav1d_open(c_out: &mut *mut Rav1dContext, s: &Rav1dSettings
     for n in 0..n_fc {
         let f: &mut Rav1dFrameData = &mut *((*c).fc).offset(n as isize);
         f.index = n;
+        addr_of_mut!(f.task_thread.tasks).write(UnsafeCell::new(Default::default()));
         addr_of_mut!(f.frame_thread).write(Default::default());
         if n_tc > 1 {
             f.task_thread.lock = Mutex::new(());
@@ -800,12 +801,10 @@ pub(crate) unsafe fn rav1d_flush(c: *mut Rav1dContext) {
         }
         let mut i_1: c_uint = 0 as c_int as c_uint;
         while i_1 < (*c).n_fc {
-            let ref mut fresh1 = (*((*c).fc).offset(i_1 as isize)).task_thread.task_head;
-            *fresh1 = 0 as *mut Rav1dTask;
-            let ref mut fresh2 = (*((*c).fc).offset(i_1 as isize)).task_thread.task_tail;
-            *fresh2 = 0 as *mut Rav1dTask;
-            let ref mut fresh3 = (*((*c).fc).offset(i_1 as isize)).task_thread.task_cur_prev;
-            *fresh3 = 0 as *mut Rav1dTask;
+            let tasks = &mut *(*((*c).fc).offset(i_1 as isize)).task_thread.tasks();
+            tasks.head = None;
+            tasks.tail = None;
+            tasks.cur_prev = None;
             *(*((*c).fc).offset(i_1 as isize))
                 .task_thread
                 .pending_tasks
@@ -913,11 +912,7 @@ impl Drop for Rav1dContext {
                 let _ = mem::take(&mut f.frame_thread); // TODO: remove when context is owned
                 mem::take(&mut f.frame_thread_progress.frame); // TODO: remove when context is owned
                 mem::take(&mut f.frame_thread_progress.copy_lpf); // TODO: remove when context is owned
-                freep(&mut f.task_thread.tasks as *mut *mut Rav1dTask as *mut c_void);
-                freep(
-                    &mut *(f.task_thread.tile_tasks).as_mut_ptr().offset(0) as *mut *mut Rav1dTask
-                        as *mut c_void,
-                );
+                mem::take(&mut f.task_thread.tasks); // TODO: remove when context is owned
                 rav1d_free_aligned(f.ts as *mut c_void);
                 rav1d_free_aligned(f.ipred_edge[0] as *mut c_void);
                 free(f.a as *mut c_void);