Skip to content

Commit

Permalink
fn rav1d_prepare_intra_edges: Make data race safe (#1144)
Browse files Browse the repository at this point in the history 
`fn rav1d_prepare_intra_edges` was already safe since we constructed the
slice before calling the `fn`, but the slice construction was `unsafe`,
and it was overlapping other `DisjointMut` `&mut` borrows. This changes
it to use `Rav1dPictureDataComponent` and thus be fully safe.

There's some duplication in the callers for the offsets and stuff.
That'll go away as I make more things fully safe regarding picture data.
  • Loading branch information
@kkysen
kkysen authored Jun 3, 2024
2 parents 3a3fbe8 + 6918872 commit a662ab4
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 81 deletions.
75 changes: 60 additions & 15 deletions src/ipred_prepare.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::include::common::bitdepth::AsPrimitive;
use crate::include::common::bitdepth::BitDepth;
use crate::include::dav1d::picture::Rav1dPictureDataComponent;
use crate::src::align::AlignedVec64;
use crate::src::const_fn::const_for;
use crate::src::disjoint_mut::DisjointMut;
Expand All @@ -24,7 +25,6 @@ use crate::src::levels::Z1_PRED;
use crate::src::levels::Z2_PRED;
use crate::src::levels::Z3_PRED;
use bitflags::bitflags;
use libc::ptrdiff_t;
use std::cmp;
use std::ffi::c_int;

Expand Down Expand Up @@ -117,6 +117,49 @@ static av1_intra_prediction_edges: [av1_intra_prediction_edge; N_IMPL_INTRA_PRED
b
};

/// Luma intra edge preparation.
///
/// `x`/`y`/`start`/`w`/`h` are in luma block (4px) units:
///
/// - `x` and `y` are the absolute block positions in the image;
/// - `start`/`w`/`h` are the *dependent tile* boundary positions.
/// In practice, `start` is the horizontal tile start,
/// `w` is the horizontal tile end,
/// the vertical tile start is assumed to be `0`,
/// and `h` is the vertical image end.
///
/// `edge_flags` signals which edges are available
/// for this transform-block inside the given partition,
/// as well as for the partition inside the superblock structure.
///
/// `dst` and `stride` are pointers to the top/left position of the current block,
/// and can be used to locate the top, left, top/left, top/right,
/// and bottom/left edge pointers also.
///
/// `angle` is the `angle_delta` `[-3..3]` on input,
/// and the absolute angle on output.
///
/// `mode` is the intra prediction mode as coded in the bitstream.
/// The return value is this same mode,
/// converted to an index in the DSP functions.
///
/// `tw`/`th` are the size of the transform block in block (4px) units.
///
/// `topleft_out` is a pointer to scratch memory
/// that will be filled with the edge pixels.
/// The memory array should have space to be indexed
/// in the `-2 * w..=2 * w` range, in the following order:
///
/// - `[0]` will be the top/left edge pixel
/// - `[1..w]` will be the top edge pixels (`1` being left-most, `w` being right-most)
/// - `[w + 1..2 * w]` will be the top/right edge pixels
/// - `[-1..-w]` will be the left edge pixels (`-1` being top-most, `-w` being bottom-most)
/// - `[-w - 1..-2 * w]` will be the bottom/left edge pixels
///
/// Each edge may remain uninitialized if it is not used by the returned mode index.
/// If edges are not available (because the edge position
/// is outside the tile dimensions or because edge_flags indicates lack of edge availability),
/// they will be extended from nearby edges as defined by the AV1 spec.
pub fn rav1d_prepare_intra_edges<BD: BitDepth>(
x: c_int,
have_left: bool,
Expand All @@ -125,8 +168,8 @@ pub fn rav1d_prepare_intra_edges<BD: BitDepth>(
w: c_int,
h: c_int,
edge_flags: EdgeFlags,
dst: &[BD::Pixel], // contains 4*h first rows of picture, last row in slice contains 4*w samples
stride: ptrdiff_t,
dst: &Rav1dPictureDataComponent,
dst_offset: usize,
// Buffer and offset pair. `isize` value is the base offset that should be used
// when indexing into the buffer.
prefilter_toplevel_sb_edge: Option<(&DisjointMut<AlignedVec64<u8>>, isize)>,
Expand All @@ -142,10 +185,7 @@ pub fn rav1d_prepare_intra_edges<BD: BitDepth>(
assert!(y < h && x < w);

let bitdepth = bd.bitdepth();
let stride = BD::pxstride(stride);

let dst_offset = 4 * x as usize
+ (if stride >= 0 { 4 * y } else { 4 * (h - y) - 1 }) as usize * stride.unsigned_abs();
let stride = dst.pixel_stride::<BD>();

match mode {
VERT_PRED..=VERT_LEFT_PRED => {
Expand Down Expand Up @@ -174,6 +214,7 @@ pub fn rav1d_prepare_intra_edges<BD: BitDepth>(

// `dst_top` starts with either the top or top-left sample depending on whether have_left is true
let edge_buf_guard;
let dst_guard;
let dst_top = if have_top
&& (av1_intra_prediction_edges[mode as usize]
.needs
Expand All @@ -190,10 +231,12 @@ pub fn rav1d_prepare_intra_edges<BD: BitDepth>(
let n = px_have + have_left as usize;
if let Some((edge_buf, base)) = prefilter_toplevel_sb_edge {
let offset = ((x * 4) as usize - have_left as usize).wrapping_add_signed(base);
edge_buf_guard = edge_buf.slice_as(offset..offset + n);
&edge_buf_guard
edge_buf_guard = edge_buf.slice_as((offset.., ..n));
&*edge_buf_guard
} else {
&dst[(dst_offset as isize - stride) as usize - have_left as usize..][..n]
let offset = dst_offset.wrapping_add_signed(-stride) - have_left as usize;
dst_guard = dst.slice::<BD, _>((offset.., ..n));
&*dst_guard
}
} else {
&[]
Expand All @@ -208,7 +251,8 @@ pub fn rav1d_prepare_intra_edges<BD: BitDepth>(
if have_left {
let px_have = cmp::min(sz, (h - y << 2) as usize);
for i in 0..px_have {
left[sz - 1 - i] = dst[(i as isize * stride + dst_offset as isize - 1) as usize];
left[sz - 1 - i] =
*dst.index::<BD>(dst_offset.wrapping_add_signed(i as isize * stride) - 1);
}
if px_have < sz {
BD::pixel_set(left, left[sz - px_have], sz - px_have);
Expand Down Expand Up @@ -237,8 +281,9 @@ pub fn rav1d_prepare_intra_edges<BD: BitDepth>(
if have_bottomleft {
let px_have = cmp::min(sz, (h - y - th << 2) as usize);
for i in 0..px_have {
bottom_left[sz - 1 - i] =
dst[((sz + i) as isize * stride + dst_offset as isize - 1) as usize];
bottom_left[sz - 1 - i] = *dst.index::<BD>(
dst_offset.wrapping_add_signed((sz + i) as isize * stride) - 1,
);
}
if px_have < sz {
BD::pixel_set(bottom_left, bottom_left[sz - px_have], sz - px_have);
Expand All @@ -265,7 +310,7 @@ pub fn rav1d_prepare_intra_edges<BD: BitDepth>(
BD::pixel_set(
top,
if have_left {
dst[dst_offset - 1]
*dst.index::<BD>(dst_offset - 1)
} else {
((1 << bitdepth >> 1) - 1).as_::<BD::Pixel>()
},
Expand Down Expand Up @@ -305,7 +350,7 @@ pub fn rav1d_prepare_intra_edges<BD: BitDepth>(
corner[1] = if have_top {
dst_top[0]
} else if have_left {
dst[dst_offset - 1]
*dst.index::<BD>(dst_offset - 1)
} else {
(1 << bitdepth >> 1).as_::<BD::Pixel>()
};
Expand Down
95 changes: 29 additions & 66 deletions src/recon.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2618,16 +2618,6 @@ pub(crate) unsafe fn rav1d_recon_b_intra<BD: BitDepth>(
.edge
.buf_mut::<BD>();
let edge_offset = 128;
let data_stride = BD::pxstride(f.cur.stride[0]);
let data_width = 4 * ts.tiling.col_end;
let data_height = 4 * ts.tiling.row_end;
let data_diff = (data_height - 1) as isize * data_stride;
let dst_slice = slice::from_raw_parts(
cur_data[0]
.as_strided_ptr::<BD>()
.offset(cmp::min(data_diff, 0)),
data_diff.unsigned_abs() + data_width as usize,
);
m = rav1d_prepare_intra_edges(
t.b.x,
t.b.x > ts.tiling.col_start,
Expand All @@ -2636,8 +2626,8 @@ pub(crate) unsafe fn rav1d_recon_b_intra<BD: BitDepth>(
ts.tiling.col_end,
ts.tiling.row_end,
edge_flags,
dst_slice,
f.cur.stride[0],
y_dst,
y_dst_offset,
top_sb_edge_slice,
intra.y_mode as IntraPredMode,
&mut angle,
Expand Down Expand Up @@ -2855,15 +2845,10 @@ pub(crate) unsafe fn rav1d_recon_b_intra<BD: BitDepth>(
let ystart = ts.tiling.row_start >> ss_ver;
let edge_array = scratch.interintra_edge_pal.edge.buf_mut::<BD>();
let edge_offset = 128;
let data_stride = BD::pxstride(f.cur.stride[1]);
let data_width = 4 * ts.tiling.col_end >> ss_hor;
let data_height = 4 * ts.tiling.row_end >> ss_ver;
let data_diff = (data_height - 1) as isize * data_stride;
let uvdst_slice = slice::from_raw_parts(
cur_data[1 + pl]
.as_strided_ptr::<BD>()
.offset(cmp::min(data_diff, 0)),
data_diff.unsigned_abs() + data_width as usize,
let uv_dst = &cur_data[1 + pl];
let uv_dst_offset = uv_dst.pixel_offset::<BD>().wrapping_add_signed(
4 * ((t.b.x >> ss_hor) as isize
+ (t.b.y >> ss_ver) as isize * uv_dst.pixel_stride::<BD>()),
);
let m: IntraPredMode = rav1d_prepare_intra_edges(
xpos,
Expand All @@ -2873,8 +2858,8 @@ pub(crate) unsafe fn rav1d_recon_b_intra<BD: BitDepth>(
ts.tiling.col_end >> ss_hor,
ts.tiling.row_end >> ss_ver,
EdgeFlags::empty(),
uvdst_slice,
stride,
uv_dst,
uv_dst_offset,
top_sb_edge_slice,
DC_PRED,
&mut angle,
Expand All @@ -2885,11 +2870,6 @@ pub(crate) unsafe fn rav1d_recon_b_intra<BD: BitDepth>(
edge_offset,
bd,
);
let uv_dst = &cur_data[1 + pl];
let uv_dst_offset = uv_dst.pixel_offset::<BD>().wrapping_add_signed(
4 * ((t.b.x >> ss_hor) as isize
+ (t.b.y >> ss_ver) as isize * uv_dst.pixel_stride::<BD>()),
);
f.dsp.ipred.cfl_pred[m as usize].call(
uv_dst,
uv_dst_offset,
Expand Down Expand Up @@ -3064,16 +3044,6 @@ pub(crate) unsafe fn rav1d_recon_b_intra<BD: BitDepth>(
.edge
.buf_mut::<BD>();
let edge_offset = 128;
let data_stride = BD::pxstride(f.cur.stride[1]);
let data_width = 4 * ts.tiling.col_end >> ss_hor;
let data_height = 4 * ts.tiling.row_end >> ss_ver;
let data_diff = (data_height - 1) as isize * data_stride;
let dstuv_slice = slice::from_raw_parts(
cur_data[1 + pl]
.as_strided_ptr::<BD>()
.offset(cmp::min(data_diff, 0)),
data_diff.unsigned_abs() + data_width as usize,
);
m = rav1d_prepare_intra_edges(
xpos,
xpos > xstart,
Expand All @@ -3082,8 +3052,8 @@ pub(crate) unsafe fn rav1d_recon_b_intra<BD: BitDepth>(
ts.tiling.col_end >> ss_hor,
ts.tiling.row_end >> ss_ver,
edge_flags,
dstuv_slice,
stride,
uv_dst,
uv_dst_offset,
top_sb_edge_slice,
uv_mode,
&mut angle,
Expand Down Expand Up @@ -3300,6 +3270,10 @@ pub(crate) unsafe fn rav1d_recon_b_inter<BD: BitDepth>(
let mut dst = cur_data[0]
.as_strided_mut_ptr::<BD>()
.offset(4 * (t.b.y as isize * BD::pxstride(f.cur.stride[0]) + t.b.x as isize));
let y_dst = &cur_data[0];
let mut y_dst_offset = y_dst
.pixel_offset::<BD>()
.wrapping_add_signed(4 * (t.b.y as isize * y_dst.pixel_stride::<BD>() + t.b.x as isize));
let uvdstoff = 4
* ((t.b.x >> ss_hor) as isize + (t.b.y >> ss_ver) as isize * BD::pxstride(f.cur.stride[1]));
let frame_hdr = &***f.frame_hdr.as_ref().unwrap();
Expand Down Expand Up @@ -3608,16 +3582,6 @@ pub(crate) unsafe fn rav1d_recon_b_inter<BD: BitDepth>(
} else {
None
};
let data_stride = BD::pxstride(f.cur.stride[0]);
let data_width = 4 * ts.tiling.col_end;
let data_height = 4 * ts.tiling.row_end;
let data_diff = (data_height - 1) as isize * data_stride;
let dst_slice = slice::from_raw_parts(
cur_data[0]
.as_strided_ptr::<BD>()
.offset(cmp::min(data_diff, 0)),
data_diff.unsigned_abs() + data_width as usize,
);
m = rav1d_prepare_intra_edges(
t.b.x,
t.b.x > ts.tiling.col_start,
Expand All @@ -3626,8 +3590,8 @@ pub(crate) unsafe fn rav1d_recon_b_inter<BD: BitDepth>(
ts.tiling.col_end,
ts.tiling.row_end,
EdgeFlags::empty(),
dst_slice,
f.cur.stride[0],
y_dst,
y_dst_offset,
top_sb_edge_slice,
m,
&mut angle,
Expand Down Expand Up @@ -3918,6 +3882,9 @@ pub(crate) unsafe fn rav1d_recon_b_inter<BD: BitDepth>(
mode => mode as IntraPredMode,
};
let mut angle = 0;
let uv_dst = &cur_data[1 + pl];
let uv_dst_offset =
uv_dst.pixel_offset::<BD>().wrapping_add_signed(uvdstoff);
let uvdst = cur_data[1 + pl].as_strided_mut_ptr::<BD>().offset(uvdstoff);
let top_sb_edge_slice = if t.b.y & f.sb_step - 1 == 0 {
let sby = t.b.y >> f.sb_shift;
Expand All @@ -3927,26 +3894,16 @@ pub(crate) unsafe fn rav1d_recon_b_inter<BD: BitDepth>(
} else {
None
};
let data_stride = BD::pxstride(f.cur.stride[1]);
let data_width = 4 * ts.tiling.col_end >> ss_hor;
let data_height = 4 * ts.tiling.row_end >> ss_ver;
let data_diff = (data_height - 1) as isize * data_stride;
let dstuv_slice = slice::from_raw_parts(
cur_data[1 + pl]
.as_strided_ptr::<BD>()
.offset(cmp::min(data_diff, 0)),
data_diff.unsigned_abs() + data_width as usize,
);
m = rav1d_prepare_intra_edges(
t.b.x >> ss_hor,
t.b.x >> ss_hor > ts.tiling.col_start >> ss_hor,
(t.b.x >> ss_hor) > (ts.tiling.col_start >> ss_hor),
t.b.y >> ss_ver,
t.b.y >> ss_ver > ts.tiling.row_start >> ss_ver,
(t.b.y >> ss_ver) > (ts.tiling.row_start >> ss_ver),
ts.tiling.col_end >> ss_hor,
ts.tiling.row_end >> ss_ver,
EdgeFlags::empty(),
dstuv_slice,
f.cur.stride[1],
uv_dst,
uv_dst_offset,
top_sb_edge_slice,
m,
&mut angle,
Expand Down Expand Up @@ -4048,6 +4005,8 @@ pub(crate) unsafe fn rav1d_recon_b_inter<BD: BitDepth>(
// coefficient coding & inverse transforms
let mut y_off = (init_y != 0) as c_int;
let mut y;
y_dst_offset =
y_dst_offset.wrapping_add_signed(y_dst.pixel_stride::<BD>() * 4 * init_y as isize);
dst = dst.offset(BD::pxstride(f.cur.stride[0]) * 4 * init_y as isize);
y = init_y;
t.b.y += init_y;
Expand All @@ -4074,12 +4033,16 @@ pub(crate) unsafe fn rav1d_recon_b_inter<BD: BitDepth>(
x += ytx.w as c_int;
x_off += 1;
}
y_dst_offset = y_dst_offset
.wrapping_add_signed(y_dst.pixel_stride::<BD>() * 4 * ytx.h as isize);
dst = dst.offset(BD::pxstride(f.cur.stride[0]) * 4 * ytx.h as isize);
t.b.x -= x;
t.b.y += ytx.h as c_int;
y += ytx.h as c_int;
y_off += 1;
}
y_dst_offset =
y_dst_offset.wrapping_add_signed(y_dst.pixel_stride::<BD>() * 4 * y as isize);
dst = dst.offset(-BD::pxstride(f.cur.stride[0]) * 4 * y as isize);
t.b.y -= y;

Expand Down

0 comments on commit a662ab4

Please sign in to comment.