Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

struct FrameTileThreadData::lowest_pixel_mem: Make into Vec #878

Merged
merged 1 commit into from
Mar 21, 2024
Merged

struct FrameTileThreadData::lowest_pixel_mem: Make into Vec #878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions src/decode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ use crate::src::thread_task::TILE_ERROR;
use crate::src::warpmv::rav1d_find_affine_int;
use crate::src::warpmv::rav1d_get_shear_params;
use crate::src::warpmv::rav1d_set_affine_mv2d;
use libc::free;
use libc::malloc;
use libc::ptrdiff_t;
use libc::uintptr_t;
Expand Down Expand Up @@ -4325,18 +4324,11 @@ pub(crate) unsafe fn rav1d_decode_frame_init(
}

let lowest_pixel_mem_sz = frame_hdr.tiling.cols * f.sbh;
if lowest_pixel_mem_sz != f.tile_thread.lowest_pixel_mem_sz {
free(f.tile_thread.lowest_pixel_mem as *mut c_void);
f.tile_thread.lowest_pixel_mem =
malloc(lowest_pixel_mem_sz as usize * ::core::mem::size_of::<[[c_int; 2]; 7]>())
as *mut [[c_int; 2]; 7];
if f.tile_thread.lowest_pixel_mem.is_null() {
f.tile_thread.lowest_pixel_mem_sz = 0;
return Err(ENOMEM);
}
f.tile_thread.lowest_pixel_mem_sz = lowest_pixel_mem_sz;
}
let mut lowest_pixel_ptr = f.tile_thread.lowest_pixel_mem;
// TODO: Fallible allocation
f.lowest_pixel_mem
.resize(lowest_pixel_mem_sz as usize, Default::default());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.resize(lowest_pixel_mem_sz as usize, Default::default());
.resize_with(lowest_pixel_mem_sz as usize, Default::default);

Copy link
Collaborator Author

@randomPoison randomPoison Mar 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why though? What's the difference between these two? The data is Copy so the end result is the same.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs recommend it for Default::default and it optimizes a little bit better, since .resize does a write for n - 1, cloning them, and then writes the last one without the clone. .resize_with just write all n at once, so the codegen is better, and the source is a bit cleaner.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted. I'll use resize_with going forward.


let mut lowest_pixel_ptr = f.lowest_pixel_mem.as_mut_ptr();
for tile_row in 0..frame_hdr.tiling.rows {
let tile_row_base = tile_row * frame_hdr.tiling.cols;
let tile_row_sb_h = frame_hdr.tiling.row_start_sb[(tile_row + 1) as usize] as c_int
Expand Down
9 changes: 1 addition & 8 deletions src/internal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,13 +517,6 @@ pub(crate) struct Rav1dFrameContext_task_thread {
pub pending_tasks: Mutex<Rav1dFrameContext_task_thread_pending_tasks>,
}

// threading (refer to tc[] for per-thread things)
#[repr(C)]
pub struct FrameTileThreadData {
pub lowest_pixel_mem: *mut [[c_int; 2]; 7],
pub lowest_pixel_mem_sz: c_int,
}

pub(crate) struct Rav1dFrameContext_frame_thread_progress {
pub entropy: AtomicI32,
pub deblock: AtomicI32, // in sby units
Expand Down Expand Up @@ -592,7 +585,7 @@ pub(crate) struct Rav1dFrameData {
pub frame_thread_progress: Rav1dFrameContext_frame_thread_progress,
pub lf: Rav1dFrameContext_lf,
pub task_thread: Rav1dFrameContext_task_thread,
pub tile_thread: FrameTileThreadData,
pub lowest_pixel_mem: Vec<[[c_int; 2]; 7]>,
}

impl Rav1dFrameData {
Expand Down
5 changes: 1 addition & 4 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,10 +902,7 @@ impl Drop for Rav1dContext {
while !(self.fc).is_null() && n_1 < self.n_fc {
let f: &mut Rav1dFrameData = &mut *(self.fc).offset(n_1 as isize);
if self.n_fc > 1 as c_uint {
freep(
&mut f.tile_thread.lowest_pixel_mem as *mut *mut [[c_int; 2]; 7]
as *mut c_void,
);
let _ = mem::take(&mut f.lowest_pixel_mem); // TODO: remove when context is owned
}
if self.tc.len() > 1 {
let _ = mem::take(&mut f.task_thread.pending_tasks); // TODO: remove when context is owned
Expand Down
Loading