`mod itx`: backport changes from dav1d 1.5.0 #1438

fbossen · 2025-06-30T01:47:10Z

Main item is "restrict number of columns iterated over based on EOB" which applies to the generic code for transforms (not asm).

…rom dav1d 1.5.0

kkysen · 2025-08-16T07:24:28Z

src/in_range.rs

+    pub const fn const_get(&'static self) -> T {
+        self.0
+    }


Suggested change

pub const fn const_get(&'static self) -> T {

self.0

}

/// This doesn't [`assert_unchecked`] that the value is in bounds (for optimization)

/// because [`Self::in_bounds`] is not a `const fn` due to its usage of `trait`s.

/// However, because this is meant to be called in a `const` context,

/// everything should be known already, and the [`assert_unchecked`]

/// should be unnecessary for optimization.

pub const fn const_get(self) -> T {

self.0

}

kkysen · 2025-08-17T02:06:48Z

src/itx.rs

+    let eob = eob as usize;
+    // in first 1d itx
+    let last_nonzero_col = if second == Identity && first != Identity {
+        std::cmp::min(sh - 1, eob)


Suggested change

std::cmp::min(sh - 1, eob)

cmp::min(sh - 1, eob)

kkysen · 2025-08-17T02:21:07Z

src/scan.rs

+    let mut last_nonzero_col_from_eob: [u8; S] = [0; S];
+
+    let mut max_col: u8 = 0;
+    const_for!(n in 0..S => {


Could we name S N instead? Unless there's a reason you used S.

kkysen · 2025-08-17T02:25:43Z

src/scan.rs

+    const_for!(n in 0..S => {
+        let rc = scan[n].const_get();
+        let rcx = (rc & (h - 1) as u16) as u8;
+        max_col = if rcx > max_col { rcx } else {max_col };


I think we have a const_max! macro. Or at least const_min! and we can add const_max!.

kkysen · 2025-08-17T02:29:31Z

src/scan.rs

+    last_nonzero_col_from_eob
+}
+
+static LAST_NONZERO_COL_FROM_EOB_4X4: [u8; 16] = init_tbl(&SCAN_4X4.0, 4);


Could you leave the array lengths still in their factored state?

kkysen · 2025-08-17T04:06:02Z

src/itx.rs

+            (Identity, 4) => rav1d_inv_identity4_1d_c,
+            (Identity, 8) => rav1d_inv_identity8_1d_c,
+            (Identity, 16) => rav1d_inv_identity16_1d_c,
+            (Identity, 32) => rav1d_inv_identity32_1d_c,
+            (Dct, 4) => rav1d_inv_dct4_1d_c,
+            (Dct, 8) => rav1d_inv_dct8_1d_c,
+            (Dct, 16) => rav1d_inv_dct16_1d_c,
+            (Dct, 32) => rav1d_inv_dct32_1d_c,
+            (Dct, 64) => rav1d_inv_dct64_1d_c,
+            (Adst, 4) => rav1d_inv_adst4_1d_c,
+            (Adst, 8) => rav1d_inv_adst8_1d_c,
+            (Adst, 16) => rav1d_inv_adst16_1d_c,
+            (FlipAdst, 4) => rav1d_inv_flipadst4_1d_c,
+            (FlipAdst, 8) => rav1d_inv_flipadst8_1d_c,
+            (FlipAdst, 16) => rav1d_inv_flipadst16_1d_c,


Can you remove the rav1d_* prefixes? I think dav1d removed its dav1d_* prefixes for all of these.

kkysen · 2025-08-17T04:22:11Z

src/scan.rs

+const fn init_tbl<const S: usize>(scan: &'static [Scan; S], h: u16) -> [u8; S] {
+    let mut last_nonzero_col_from_eob: [u8; S] = [0; S];
+
+    let mut max_col: u8 = 0;
+    const_for!(n in 0..S => {
+        let rc = scan[n].const_get();
+        let rcx = (rc & (h - 1) as u16) as u8;
+        max_col = if rcx > max_col { rcx } else {max_col };
+        last_nonzero_col_from_eob[n] = max_col;
+    });
+
+    last_nonzero_col_from_eob
+}


Suggested change

const fn init_tbl<const S: usize>(scan: &'static [Scan; S], h: u16) -> [u8; S] {

let mut last_nonzero_col_from_eob: [u8; S] = [0; S];

let mut max_col: u8 = 0;

const_for!(n in 0..S => {

let rc = scan[n].const_get();

let rcx = (rc & (h - 1) as u16) as u8;

max_col = if rcx > max_col { rcx } else {max_col };

last_nonzero_col_from_eob[n] = max_col;

});

last_nonzero_col_from_eob

}

const fn init_tbl<const N: usize>(scan: &'static [Scan; N], h: u16) -> [u8; N] {

let mut last_nonzero_col_from_eob = [0; N];

let mut max_col = 0;

const_for!(n in 0..N => {

let rc = scan[n].const_get();

let rcx = (rc & (h - 1)) as u8;

max_col = const_max!(max_col, rcx);

last_nonzero_col_from_eob[n] = max_col;

});

last_nonzero_col_from_eob

}

with

macro_rules! const_max { ($a:expr, $b:expr) => {{ let a = $a; let b = $b; if a < b { a } else { b } }}; } /// [`std::cmp::max`] is not `const` since it would need `const` `trait` `fn`s, /// so this implements it with a macro instead so it can remain `const`. pub(crate) use const_max;

kkysen · 2025-08-17T04:38:07Z

src/scan.rs

+pub static DAV1D_LAST_NONZERO_COL_FROM_EOB: [&'static [u8]; TxfmSize::COUNT] = [
+    &LAST_NONZERO_COL_FROM_EOB_4X4,
+    &LAST_NONZERO_COL_FROM_EOB_8X8,
+    &LAST_NONZERO_COL_FROM_EOB_16X16,
+    &LAST_NONZERO_COL_FROM_EOB_32X32,
+    &LAST_NONZERO_COL_FROM_EOB_32X32,
+    &LAST_NONZERO_COL_FROM_EOB_4X8,
+    &LAST_NONZERO_COL_FROM_EOB_8X4,
+    &LAST_NONZERO_COL_FROM_EOB_8X16,
+    &LAST_NONZERO_COL_FROM_EOB_16X8,
+    &LAST_NONZERO_COL_FROM_EOB_16X32,
+    &LAST_NONZERO_COL_FROM_EOB_32X16,
+    &LAST_NONZERO_COL_FROM_EOB_32X32,
+    &LAST_NONZERO_COL_FROM_EOB_32X32,
+    &LAST_NONZERO_COL_FROM_EOB_4X16,
+    &LAST_NONZERO_COL_FROM_EOB_16X4,
+    &LAST_NONZERO_COL_FROM_EOB_8X32,
+    &LAST_NONZERO_COL_FROM_EOB_32X8,
+    &LAST_NONZERO_COL_FROM_EOB_16X32,
+    &LAST_NONZERO_COL_FROM_EOB_32X16,
+];


It's better to use enum_map! here so that the order isn't implicit.

Suggested change

pub static DAV1D_LAST_NONZERO_COL_FROM_EOB: [&'static [u8]; TxfmSize::COUNT] = [

&LAST_NONZERO_COL_FROM_EOB_4X4,

&LAST_NONZERO_COL_FROM_EOB_8X8,

&LAST_NONZERO_COL_FROM_EOB_16X16,

&LAST_NONZERO_COL_FROM_EOB_32X32,

&LAST_NONZERO_COL_FROM_EOB_32X32,

&LAST_NONZERO_COL_FROM_EOB_4X8,

&LAST_NONZERO_COL_FROM_EOB_8X4,

&LAST_NONZERO_COL_FROM_EOB_8X16,

&LAST_NONZERO_COL_FROM_EOB_16X8,

&LAST_NONZERO_COL_FROM_EOB_16X32,

&LAST_NONZERO_COL_FROM_EOB_32X16,

&LAST_NONZERO_COL_FROM_EOB_32X32,

&LAST_NONZERO_COL_FROM_EOB_32X32,

&LAST_NONZERO_COL_FROM_EOB_4X16,

&LAST_NONZERO_COL_FROM_EOB_16X4,

&LAST_NONZERO_COL_FROM_EOB_8X32,

&LAST_NONZERO_COL_FROM_EOB_32X8,

&LAST_NONZERO_COL_FROM_EOB_16X32,

&LAST_NONZERO_COL_FROM_EOB_32X16,

];

pub static RAV1D_LAST_NONZERO_COL_FROM_EOB: enum_map_ty!(TxfmSize, &'static [u8]) = enum_map!(TxfmSize => &'static [u8]; match key {

S4x4 => LAST_NONZERO_COL_FROM_EOB_4X4.as_slice(),

S8x8 => LAST_NONZERO_COL_FROM_EOB_8X8.as_slice(),

S16x16 => LAST_NONZERO_COL_FROM_EOB_16X16.as_slice(),

S32x32 => LAST_NONZERO_COL_FROM_EOB_32X32.as_slice(),

S64x64 => LAST_NONZERO_COL_FROM_EOB_32X32.as_slice(),

R4x8 => LAST_NONZERO_COL_FROM_EOB_4X8.as_slice(),

R8x4 => LAST_NONZERO_COL_FROM_EOB_8X4.as_slice(),

R8x16 => LAST_NONZERO_COL_FROM_EOB_8X16.as_slice(),

R16x8 => LAST_NONZERO_COL_FROM_EOB_16X8.as_slice(),

R16x32 => LAST_NONZERO_COL_FROM_EOB_16X32.as_slice(),

R32x16 => LAST_NONZERO_COL_FROM_EOB_32X16.as_slice(),

R32x64 => LAST_NONZERO_COL_FROM_EOB_32X32.as_slice(),

R64x32 => LAST_NONZERO_COL_FROM_EOB_32X32.as_slice(),

R4x16 => LAST_NONZERO_COL_FROM_EOB_4X16.as_slice(),

R16x4 => LAST_NONZERO_COL_FROM_EOB_16X4.as_slice(),

R8x32 => LAST_NONZERO_COL_FROM_EOB_8X32.as_slice(),

R32x8 => LAST_NONZERO_COL_FROM_EOB_32X8.as_slice(),

R16x64 => LAST_NONZERO_COL_FROM_EOB_16X32.as_slice(),

R64x16 => LAST_NONZERO_COL_FROM_EOB_32X16.as_slice(),

});

This requires a couple extra small impls, so I can add those if you want.

Also, we should name it RAV1D_*.

Splitting out of the large #1438.

fbossen force-pushed the bp-1.5.0-0011 branch 2 times, most recently from 08f9149 to 036b750 Compare June 30, 2025 02:45

kkysen self-requested a review July 7, 2025 15:49

kkysen force-pushed the bp-1.5.0-0011 branch from 036b750 to 173a4dd Compare August 16, 2025 06:31

This was referenced Aug 16, 2025

mod itx: backport deduplicating itx macros #1450

Merged

fn dav1d_inv_wht4_1d_c: backport trimming #1451

Merged

kkysen added the backports Backports from dav1d label Aug 16, 2025

kkysen changed the base branch from main to kkysen/backport-trim-dav1d_inv_wht4_1d_c August 16, 2025 06:37

kkysen force-pushed the kkysen/backport-trim-dav1d_inv_wht4_1d_c branch from c7de0bb to 6c66002 Compare August 16, 2025 06:55

kkysen force-pushed the bp-1.5.0-0011 branch from 173a4dd to db89b12 Compare August 16, 2025 06:55

rbultje and others added 2 commits August 16, 2025 00:09

itx: restrict number of columns iterated over based on EOB

0d8e17a

itx: backport restrict number of columns iterated over based on EOB f…

a94e71c

…rom dav1d 1.5.0

kkysen force-pushed the kkysen/backport-trim-dav1d_inv_wht4_1d_c branch from 6c66002 to 595b3da Compare August 16, 2025 07:14

kkysen force-pushed the bp-1.5.0-0011 branch from db89b12 to a94e71c Compare August 16, 2025 07:14

kkysen reviewed Aug 16, 2025

View reviewed changes

kkysen requested changes Aug 17, 2025

View reviewed changes

kkysen added a commit that referenced this pull request Aug 26, 2025

mod itx: backport deduplicating itx macros (#1450)

1c21a71

Splitting out of the large #1438.

kkysen added a commit that referenced this pull request Aug 26, 2025

fn dav1d_inv_wht4_1d_c: backport trimming (#1451)

f699aef

Splitting out of the large #1438.

kkysen deleted the branch memorysafety:kkysen/backport-trim-dav1d_inv_wht4_1d_c August 26, 2025 17:45

kkysen closed this Aug 26, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`mod itx`: backport changes from dav1d 1.5.0 #1438

`mod itx`: backport changes from dav1d 1.5.0 #1438

Uh oh!

fbossen commented Jun 30, 2025

Uh oh!

kkysen Aug 16, 2025

Uh oh!

kkysen Aug 17, 2025

Uh oh!

kkysen Aug 17, 2025

Uh oh!

kkysen Aug 17, 2025

Uh oh!

kkysen Aug 17, 2025

Uh oh!

kkysen Aug 17, 2025

Uh oh!

kkysen Aug 17, 2025

Uh oh!

kkysen Aug 17, 2025 •

edited

Loading

Uh oh!

Uh oh!

-    pub const fn const_get(&'static self) -> T {
-        self.0
-    }
+    /// This doesn't [`assert_unchecked`] that the value is in bounds (for optimization)
+    /// because [`Self::in_bounds`] is not a `const fn` due to its usage of `trait`s.
+    /// However, because this is meant to be called in a `const` context,
+    /// everything should be known already, and the [`assert_unchecked`]
+    /// should be unnecessary for optimization.
+    pub const fn const_get(self) -> T {
+        self.0
+    }

mod itx: backport changes from dav1d 1.5.0 #1438

mod itx: backport changes from dav1d 1.5.0 #1438

Uh oh!

Conversation

fbossen commented Jun 30, 2025

Uh oh!

kkysen Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

kkysen Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

kkysen Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

kkysen Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

kkysen Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

kkysen Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

kkysen Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

kkysen Aug 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

`mod itx`: backport changes from dav1d 1.5.0 #1438

`mod itx`: backport changes from dav1d 1.5.0 #1438

kkysen Aug 17, 2025 •

edited

Loading