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

COM-6142 #187

Merged
merged 1 commit into from
Oct 17, 2024
Merged

COM-6142 #187

merged 1 commit into from
Oct 17, 2024

Conversation

mcesariniflu
Copy link
Contributor

modified:   fluster/test_suite.py
modified:   fluster/utils.py
new file:   scripts/gen_av1_argon.py
new file:   scripts/test_zip/SVCBC-1/SVCBC-1-L0.264
new file:   scripts/test_zip/SVCBC-1/SVCBC-1-L1.264
new file:   scripts/test_zip/SVCBC-1/SVCBC-1.264
new file:   test_suites/av1/AV1_ARGON_VECTORS_MANUAL.json

@mdimopoulos mdimopoulos marked this pull request as ready for review October 3, 2024 11:52
@mdimopoulos mdimopoulos self-requested a review October 4, 2024 10:53
@mdimopoulos
Copy link
Contributor

mdimopoulos commented Oct 4, 2024

The foundations seem solid to me. I have several change requests though to make the code neater. See individual comments


Important time stats

  • scripts/gen_av1_argon.py -> took about 30 minutes to download, extract, read checksums and create the test suite
  • ./fluster.py download AV1_ARGON_VECTORS -> after having created the test suite file from above step, downloading the source file and extracting the vectors took 2h and 47 minutes

So in a future iteration we should try to bring the download logic of fluster close to the one used in the test suite generator

scripts/test_zip/SVCBC-1/SVCBC-1-L0.264 Outdated Show resolved Hide resolved
scripts/test_zip/SVCBC-1/SVCBC-1-L1.264 Outdated Show resolved Hide resolved
scripts/test_zip/SVCBC-1/SVCBC-1.264 Outdated Show resolved Hide resolved
fluster/test_suite.py Outdated Show resolved Hide resolved
fluster/test_suite.py Show resolved Hide resolved
scripts/gen_av1_argon.py Outdated Show resolved Hide resolved
scripts/gen_av1_argon.py Outdated Show resolved Hide resolved
scripts/gen_av1_argon.py Outdated Show resolved Hide resolved
scripts/gen_av1_argon.py Outdated Show resolved Hide resolved
fluster/utils.py Outdated Show resolved Hide resolved
@mdimopoulos mdimopoulos self-requested a review October 17, 2024 11:12
Copy link
Contributor

@mdimopoulos mdimopoulos left a comment

Choose a reason for hiding this comment

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

All pending points have been addressed 👍🏼.

I encountered issues running gen_av1_argon.py successfully on MacOS. I decided to push a fixup commit that solves any problems.
@mcesariniflu let me know if the latest version of the script runs well on Ubuntu.

- created new test suite generator script
- modified test suite structure to accomodate this special case
- added the generated test suite
- tested to work on Ubuntu 20, 22 and MacOS 14.4
Copy link
Contributor

@mdimopoulos mdimopoulos left a comment

Choose a reason for hiding this comment

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

I tried that it works fine on Ubuntu 20. Approved

@mdimopoulos mdimopoulos merged commit 9ac8828 into master Oct 17, 2024
3 checks passed
@mdimopoulos mdimopoulos deleted the COM-6142 branch October 17, 2024 15: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.

3 participants