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 Rav1dIntraPredDspContext::cfl_ac: Make an EnumMap over Rav1dPixelLayoutSubSampled #585

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

kkysen
Copy link
Collaborator

@kkysen kkysen commented Nov 21, 2023

I'll do the same for the other array fields, but I need to make the enums for those first; this one already exists from before.

@kkysen kkysen requested a review from randomPoison November 21, 2023 23:50
@kkysen kkysen force-pushed the kkysen/cleanup-fns-intra_pred_dsp_init branch from a37dba0 to 4a60b90 Compare November 27, 2023 06:27
@kkysen kkysen force-pushed the kkysen/struct-Rav1dIntraPredDSPContext-cfl_ac-enum_map branch from 8e3b4df to 61e77a0 Compare November 27, 2023 06:28
@kkysen kkysen force-pushed the kkysen/cleanup-fns-intra_pred_dsp_init branch from 4a60b90 to eccc29a Compare November 29, 2023 07:18
@kkysen kkysen force-pushed the kkysen/struct-Rav1dIntraPredDSPContext-cfl_ac-enum_map branch from 61e77a0 to f54bf19 Compare November 29, 2023 07:18
@kkysen kkysen force-pushed the kkysen/cleanup-fns-intra_pred_dsp_init branch from eccc29a to 86d472d Compare November 29, 2023 07:44
@kkysen kkysen force-pushed the kkysen/struct-Rav1dIntraPredDSPContext-cfl_ac-enum_map branch from f54bf19 to d18b3a5 Compare November 29, 2023 07:47
@kkysen kkysen force-pushed the kkysen/cleanup-fns-intra_pred_dsp_init branch from 86d472d to 91c1922 Compare November 30, 2023 07:05
@kkysen kkysen force-pushed the kkysen/struct-Rav1dIntraPredDSPContext-cfl_ac-enum_map branch from d18b3a5 to 0e08885 Compare December 1, 2023 12:06
Copy link
Collaborator

@rinon rinon left a comment

Choose a reason for hiding this comment

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

This change itself looks fine.

However, what does this actually enable with regards to safety improvements? Was this actually blocking anything or is this a readability improvement? We need to focus on only what is blocking memory safety improvements.

@kkysen
Copy link
Collaborator Author

kkysen commented Dec 2, 2023

However, what does this actually enable with regards to safety improvements? Was this actually blocking anything or is this a readability improvement? We need to focus on only what is blocking memory safety improvements.

This was to remove the bounds check, since enums are restricted to only their values. Since we're now focusing on getting everything safe first before ensuring we don't have perf regressions, I'll hold off on these types of changes until we get everything safe. The improvement in readability/type safety is nice here, but was more of a secondary concern.

@kkysen kkysen force-pushed the kkysen/cleanup-fns-intra_pred_dsp_init branch from 91c1922 to dab1eef Compare December 2, 2023 05:04
@kkysen kkysen force-pushed the kkysen/struct-Rav1dIntraPredDSPContext-cfl_ac-enum_map branch from 0e08885 to f383eac Compare December 2, 2023 05:04
Base automatically changed from kkysen/cleanup-fns-intra_pred_dsp_init to main December 6, 2023 04:46
@kkysen kkysen merged commit 61ca0d9 into main Dec 6, 2023
18 checks passed
@kkysen kkysen deleted the kkysen/struct-Rav1dIntraPredDSPContext-cfl_ac-enum_map branch December 6, 2023 04:46
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