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

Non-persistent input in incremental decoding test #1114

Merged
merged 3 commits into from
Sep 18, 2022

Conversation

y-guyon
Copy link
Collaborator

@y-guyon y-guyon commented Sep 16, 2022

No description provided.

Copy link
Collaborator

@wantehchang wantehchang left a comment

Choose a reason for hiding this comment

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

LGTM.

tests/gtest/avifincrtest_helpers.cc Outdated Show resolved Hide resolved
tests/gtest/avifincrtest_helpers.cc Outdated Show resolved Hide resolved
tests/gtest/avifincrtest_helpers.cc Outdated Show resolved Hide resolved
tests/gtest/avifincrtest_helpers.cc Outdated Show resolved Hide resolved
@@ -259,7 +279,8 @@ void DecodeIncrementally(const avifRWData& encoded_avif, bool is_persistent,

// Emulate a byte-by-byte stream.
PartialData data = {/*available=*/{encoded_avif.data, 0},
/*fullSize=*/encoded_avif.size};
/*fullSize=*/encoded_avif.size,
/*buffer=*/{}, /*swapped_buffer=*/{}};
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMPORTANT: The initializer needs to be updated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am surprised that this doesn't break compilation. I guess I haven't mastered C++.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(2) applies here, I believe: https://en.cppreference.com/w/cpp/language/zero_initialization

clang-tidy should catch the incorrect comment though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

clang-tidy should catch the incorrect comment though.

Actually maybe that only works on function parameters.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am surprised that this doesn't break compilation. I guess I haven't mastered C++.

It is only the comments /*buffer=*/ and /*swapped_buffer=*/ that were wrong. nonpersistent_bytes was correctly set to {} (meaning nullptr) and num_nonpersistent_bytes to {} (meaning 0).

clang-tidy should catch the incorrect comment though.

Maybe we could have a clang-tidy test as part of the GitHub CI.

@y-guyon y-guyon merged commit 7f2156d into AOMediaCodec:main Sep 18, 2022
@y-guyon y-guyon deleted the fuzz_avifparse branch September 18, 2022 09:48
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