struct Rav1dContext::frame_flags: Make PictureFlags `Atomic<Pictu…

…reFlags>` (#679) A cheaper `Atomic` alternative to the `RwLock` in #673 (see #673 (comment)).
memorysafety · Jan 20, 2024 · 6040aac · 6040aac
2 parents ad1d9f2 + e235ade
commit 6040aac
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 10 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -24,6 +24,7 @@ path = "tests/seek_stress.rs"
 name = "seek_stress"
 
 [dependencies]
+atomig = { version = "0.4.0", features = ["derive"] }
 bitflags = "2.4.0"
 cfg-if = "1.0.0"
 libc = "0.2"

diff --git a/src/internal.rs b/src/internal.rs
@@ -74,6 +74,7 @@ use crate::src::refmvs::refmvs_temporal_block;
 use crate::src::refmvs::refmvs_tile;
 use crate::src::refmvs::Rav1dRefmvsDSPContext;
 use crate::src::thread_data::thread_data;
+use atomig::Atomic;
 use libc::pthread_cond_t;
 use libc::pthread_mutex_t;
 use libc::ptrdiff_t;
@@ -253,7 +254,7 @@ pub struct Rav1dContext {
     pub(crate) inloop_filters: Rav1dInloopFilterType,
     pub(crate) decode_frame_type: Rav1dDecodeFrameType,
     pub(crate) drain: c_int,
-    pub(crate) frame_flags: PictureFlags,
+    pub(crate) frame_flags: Atomic<PictureFlags>,
     pub(crate) event_flags: Rav1dEventFlags,
     pub(crate) cached_error_props: Rav1dDataProps,
     pub(crate) cached_error: Rav1dResult,

diff --git a/src/obu.rs b/src/obu.rs
@@ -2258,7 +2258,8 @@ unsafe fn parse_obus(
             match &c.seq_hdr {
                 None => {
                     c.frame_hdr = None;
-                    c.frame_flags |= PictureFlags::NEW_SEQUENCE;
+                    c.frame_flags
+                        .fetch_or(PictureFlags::NEW_SEQUENCE, Ordering::Relaxed);
                 }
                 Some(c_seq_hdr) if !seq_hdr.eq_without_operating_parameter_info(&c_seq_hdr) => {
                     // See 7.5, `operating_parameter_info` is allowed to change in
@@ -2274,13 +2275,15 @@ unsafe fn parse_obus(
                         rav1d_ref_dec(&mut c.refs[i as usize].refmvs);
                         rav1d_cdf_thread_unref(&mut c.cdf[i as usize]);
                     }
-                    c.frame_flags |= PictureFlags::NEW_SEQUENCE;
+                    c.frame_flags
+                        .fetch_or(PictureFlags::NEW_SEQUENCE, Ordering::Relaxed);
                 }
                 Some(c_seq_hdr)
                     if seq_hdr.operating_parameter_info != c_seq_hdr.operating_parameter_info =>
                 {
                     // If operating_parameter_info changed, signal it
-                    c.frame_flags |= PictureFlags::NEW_OP_PARAMS_INFO;
+                    c.frame_flags
+                        .fetch_or(PictureFlags::NEW_OP_PARAMS_INFO, Ordering::Relaxed);
                 }
                 _ => {}
             }
@@ -2465,7 +2468,10 @@ unsafe fn parse_obus(
                 }
             }
         }
-        RAV1D_OBU_TD => c.frame_flags |= PictureFlags::NEW_TEMPORAL_UNIT,
+        RAV1D_OBU_TD => {
+            c.frame_flags
+                .fetch_or(PictureFlags::NEW_TEMPORAL_UNIT, Ordering::Relaxed);
+        }
         RAV1D_OBU_PADDING => {} // Ignore OBUs we don't care about.
         _ => {
             // Print a warning, but don't fail for unknown types.

diff --git a/src/picture.rs b/src/picture.rs
@@ -31,6 +31,8 @@ use crate::src::mem::Rav1dMemPoolBuffer;
 use crate::src::r#ref::rav1d_ref_dec;
 use crate::src::r#ref::rav1d_ref_inc;
 use crate::src::r#ref::rav1d_ref_wrap;
+use atomig::Atom;
+use atomig::AtomLogic;
 use bitflags::bitflags;
 use libc::free;
 use libc::malloc;
@@ -42,12 +44,15 @@ use std::mem;
 use std::ptr;
 use std::slice;
 use std::sync::atomic::AtomicU32;
+use std::sync::atomic::Ordering;
 use std::sync::Arc;
 use to_method::To as _;
 
+#[derive(Clone, Copy, PartialEq, Eq, Hash, Default, Atom, AtomLogic)]
+pub struct PictureFlags(u8);
+
 bitflags! {
-    #[derive(Clone, Copy, PartialEq, Eq, Hash, Default)]
-    pub struct PictureFlags: u8 {
+    impl PictureFlags: u8 {
         const NEW_SEQUENCE = 1 << 0;
         const NEW_OP_PARAMS_INFO = 1 << 1;
         const NEW_TEMPORAL_UNIT = 1 << 2;
@@ -254,8 +259,7 @@ pub(crate) unsafe fn rav1d_thread_picture_alloc(
     } else {
         PictureFlags::NEW_SEQUENCE | PictureFlags::NEW_OP_PARAMS_INFO
     };
-    p.flags = c.frame_flags;
-    c.frame_flags &= flags_mask;
+    p.flags = c.frame_flags.fetch_and(flags_mask, Ordering::Relaxed);
     p.visible = frame_hdr.show_frame != 0;
     p.showable = frame_hdr.showable_frame != 0;
     p.progress = if have_frame_mt {