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

fn dav1d_parse_sequence_header: backport simplification from dav1d 1.2.1 #897

Merged
merged 4 commits into from
Apr 11, 2024

Conversation

fbossen
Copy link
Collaborator

@fbossen fbossen commented Mar 23, 2024

Avoid creating an entire decoder instance to just parse a sequence header.

src/obu.rs Outdated Show resolved Hide resolved
src/obu.rs Show resolved Hide resolved
src/obu.rs Show resolved Hide resolved
src/obu.rs Outdated Show resolved Hide resolved
src/obu.rs Outdated Show resolved Hide resolved
src/obu.c Show resolved Hide resolved
src/obu.c Show resolved Hide resolved
@fbossen fbossen force-pushed the bp-1.2.1-0021 branch 4 times, most recently from b806651 to 8e2ebfb Compare March 23, 2024 16:41
@fbossen
Copy link
Collaborator Author

fbossen commented Mar 23, 2024

Note sure whether the API function dav1d_parse_sequence_header is tested in CI. Ended up using -s 5 when running the decoder to test.

@fbossen fbossen force-pushed the bp-1.2.1-0021 branch 2 times, most recently from b2918a2 to df3dfa7 Compare March 23, 2024 23:25
@kkysen kkysen self-requested a review March 31, 2024 08:59
Copy link
Collaborator

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

LGTM! Could you just use assert! instead of assert_eq! on a bool?

src/obu.rs Outdated Show resolved Hide resolved
gramner-twoorioles and others added 3 commits April 3, 2024 12:37
It's not used by anything, and the data it references is stack-allocated.
Creating an entire decoder instance just for some bitstream parsing
is completely unnecessary. We can instead parse the sequence header
directly into the user-provided buffer while ignoring/skipping
other OBU types, with zero memory allocations required.
Copy link
Collaborator

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

Sorry I missed these couple things before. We should port the NOINLINE C added.

src/getbits.rs Outdated Show resolved Hide resolved
src/getbits.rs Outdated Show resolved Hide resolved
src/obu.rs Show resolved Hide resolved
@kkysen kkysen merged commit 10404a7 into memorysafety:main Apr 11, 2024
19 checks passed
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.

3 participants