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.

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

struct {D,R}av1d{Sequence,Frame}Header: Replace Rav1dRefs with Option<Arc<DRav1d<_, _>>>s #663

Merged
merged 3 commits into from
Jan 11, 2024

Conversation

kkysen
Copy link
Collaborator

@kkysen kkysen commented Jan 3, 2024

As these types are heavily used unlike the other Rav1dRefed types, impl Deref for DRav1d is added, derefing to R as that's the type used primarily in Rust.

And to simplify uses and avoid triple derefs (including an Arc deref and Option::unwrap) at use sites, a let {frame,seq}_hdr = &***{f,c}.{frame,seq}_hdr.as_ref().unwrap(); is refactored out in each fn where it's used.

Furthermore, similarly to #661, .update_rav1d()s are skipped. Although these types are indeed read, they are meant to be read-only, so this should be okay to do.

Unlike the previous Arcified types in #659 and #661, these Rav1dRefs were pooled, which the Arc replacement is not. However, those pools were not thread-local, so it's not clear if it is a perf regression to remove them (we'll check). Either way, the API of Arc should be similar enough to a pooled Arc (e.x. refpool) that we can switch to it later if perf demands it.

@kkysen kkysen requested review from rinon and randomPoison January 3, 2024 11:34
@kkysen kkysen force-pushed the kkysen/struct-Rav1dWarpedMotionParams-mutate-through-atomics branch from 4c5076a to cd8f316 Compare January 3, 2024 21:02
@kkysen kkysen force-pushed the kkysen/struct-Rav1dSequence-FrameHeader-Arcify branch 3 times, most recently from 46db563 to 37c1fc8 Compare January 3, 2024 21:15
@kkysen kkysen force-pushed the kkysen/struct-Rav1dWarpedMotionParams-mutate-through-atomics branch from cd8f316 to 88d9187 Compare January 9, 2024 01:19
@kkysen kkysen force-pushed the kkysen/struct-Rav1dSequence-FrameHeader-Arcify branch from 5ed3edb to da55b98 Compare January 9, 2024 01:19
@kkysen kkysen force-pushed the kkysen/struct-Rav1dSequence-FrameHeader-Arcify branch from da55b98 to 27defb4 Compare January 9, 2024 21:38
Base automatically changed from kkysen/struct-Rav1dWarpedMotionParams-mutate-through-atomics to main January 11, 2024 00:15
…ption<Arc<DRav1d<_, _>>>`s.

As these types are heavily used unlike the other `Rav1dRef`ed types,
`impl Deref for DRav1d` is added, `deref`ing to `R` as that's the type used primarily in Rust.
And to simplify uses and avoid triple derefs (including an `Arc` deref and `Option::unwrap`) at use sites,
a `let {frame,seq}_hdr = &***{f,c}.{frame,seq}_hdr.as_ref().unwrap();` is refactored out in each `fn` where it's used.
…` conversion by returning the full `DRav1d` instead of dropping the `D`.
@kkysen kkysen force-pushed the kkysen/struct-Rav1dSequence-FrameHeader-Arcify branch from 27defb4 to ca294df Compare January 11, 2024 00:20
@kkysen kkysen merged commit 120ae32 into main Jan 11, 2024
34 checks passed
@kkysen kkysen deleted the kkysen/struct-Rav1dSequence-FrameHeader-Arcify branch January 11, 2024 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants