LJpegDecompressor: change how we deal with predictor #702
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For predictor mode 1, which is the only one supported currently, the predictor for the next MCU is the fully decoded previous MCU. When going to the next row, we use the first MCU of the previous row, or the initial predictor, when decoding the first row. As an effect, we can either (a) keep decoded MCU (as new predictor) after decoding and storing it, or (b) load the previous MCU as the new predictor when needed. We were doing (a), but that won't work for other predictor modes. ``` Comparing /home/lebedevri/rawspeed/build-Clang18-release/src/utilities/rsbench/rsbench-old to /home/lebedevri/rawspeed/build-Clang18-release/src/utilities/rsbench/rsbench Benchmark Time CPU Time Old Time New CPU Old CPU New -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- ./Adobe DNG Converter/Canon EOS 5D Mark III/5G4A9394-compressed-lossless.DNG/threads:32/process_time/real_time_pvalue 0.0000 0.0000 U Test, Repetitions: 49 vs 49 ./Adobe DNG Converter/Canon EOS 5D Mark III/5G4A9394-compressed-lossless.DNG/threads:32/process_time/real_time_mean +0.0300 +0.0302 11 12 360 371 ./Adobe DNG Converter/Canon EOS 5D Mark III/5G4A9394-compressed-lossless.DNG/threads:32/process_time/real_time_median +0.0264 +0.0253 11 12 360 369 ./Adobe DNG Converter/Canon EOS 5D Mark III/5G4A9394-compressed-lossless.DNG/threads:32/process_time/real_time_stddev -0.0646 -0.0590 0 0 9 9 ./Adobe DNG Converter/Canon EOS 5D Mark III/5G4A9394-compressed-lossless.DNG/threads:32/process_time/real_time_cv -0.0918 -0.0865 0 0 0 0 ./Adobe DNG Converter/Canon EOS 5D Mark III/5G4A9395-compressed-lossless.DNG/threads:32/process_time/real_time_pvalue 0.0035 0.0045 U Test, Repetitions: 49 vs 49 ./Adobe DNG Converter/Canon EOS 5D Mark III/5G4A9395-compressed-lossless.DNG/threads:32/process_time/real_time_mean +0.0137 +0.0132 15 15 463 470 ./Adobe DNG Converter/Canon EOS 5D Mark III/5G4A9395-compressed-lossless.DNG/threads:32/process_time/real_time_median +0.0145 +0.0128 14 15 459 465 ./Adobe DNG Converter/Canon EOS 5D Mark III/5G4A9395-compressed-lossless.DNG/threads:32/process_time/real_time_stddev +0.0528 +0.0702 0 0 11 12 ./Adobe DNG Converter/Canon EOS 5D Mark III/5G4A9395-compressed-lossless.DNG/threads:32/process_time/real_time_cv +0.0386 +0.0562 0 0 0 0 ./Adobe DNG Converter/Canon EOS 5D Mark IV/B13A0729.dng/threads:32/process_time/real_time_pvalue 0.4556 0.5649 U Test, Repetitions: 49 vs 49 ./Adobe DNG Converter/Canon EOS 5D Mark IV/B13A0729.dng/threads:32/process_time/real_time_mean +0.0019 +0.0013 29 29 919 920 ./Adobe DNG Converter/Canon EOS 5D Mark IV/B13A0729.dng/threads:32/process_time/real_time_median +0.0121 +0.0108 29 29 908 917 ./Adobe DNG Converter/Canon EOS 5D Mark IV/B13A0729.dng/threads:32/process_time/real_time_stddev -0.0614 -0.0467 1 1 20 19 ./Adobe DNG Converter/Canon EOS 5D Mark IV/B13A0729.dng/threads:32/process_time/real_time_cv -0.0632 -0.0480 0 0 0 0 ./Fujifilm/X100S/fujifilm-x100s-daylight-DSCF9505.dng/threads:32/process_time/real_time_pvalue 0.0000 0.0000 U Test, Repetitions: 49 vs 49 ./Fujifilm/X100S/fujifilm-x100s-daylight-DSCF9505.dng/threads:32/process_time/real_time_mean -0.0545 -0.0544 4 4 132 125 ./Fujifilm/X100S/fujifilm-x100s-daylight-DSCF9505.dng/threads:32/process_time/real_time_median -0.0533 -0.0533 4 4 131 124 ./Fujifilm/X100S/fujifilm-x100s-daylight-DSCF9505.dng/threads:32/process_time/real_time_stddev -0.0611 -0.0586 0 0 3 3 ./Fujifilm/X100S/fujifilm-x100s-daylight-DSCF9505.dng/threads:32/process_time/real_time_cv -0.0070 -0.0045 0 0 0 0 ./Samsung/Galaxy S21 Ultra/20230712_115041.dng/threads:32/process_time/real_time_pvalue 0.0000 0.0000 U Test, Repetitions: 49 vs 49 ./Samsung/Galaxy S21 Ultra/20230712_115041.dng/threads:32/process_time/real_time_mean -0.0094 -0.0178 144 142 145 143 ./Samsung/Galaxy S21 Ultra/20230712_115041.dng/threads:32/process_time/real_time_median -0.0094 -0.0094 144 142 144 142 ./Samsung/Galaxy S21 Ultra/20230712_115041.dng/threads:32/process_time/real_time_stddev -0.4346 -0.7495 0 0 12 3 ./Samsung/Galaxy S21 Ultra/20230712_115041.dng/threads:32/process_time/real_time_cv -0.4292 -0.7450 0 0 0 0 ./Samsung/Galaxy S23 Ultra/20231214_130645.dng/threads:32/process_time/real_time_pvalue 0.1472 0.0922 U Test, Repetitions: 49 vs 49 ./Samsung/Galaxy S23 Ultra/20231214_130645.dng/threads:32/process_time/real_time_mean -0.0006 +0.0032 555 554 7061 7084 ./Samsung/Galaxy S23 Ultra/20231214_130645.dng/threads:32/process_time/real_time_median +0.0014 +0.0040 554 555 7184 7213 ./Samsung/Galaxy S23 Ultra/20231214_130645.dng/threads:32/process_time/real_time_stddev -0.4211 +0.0266 3 2 855 878 ./Samsung/Galaxy S23 Ultra/20231214_130645.dng/threads:32/process_time/real_time_cv -0.4208 +0.0233 0 0 0 0 ./Sony/ILCE-7RM5/7RM5-S35-LosslessCompressedMedium.ARW/threads:32/process_time/real_time_pvalue 0.0121 0.0101 U Test, Repetitions: 49 vs 49 ./Sony/ILCE-7RM5/7RM5-S35-LosslessCompressedMedium.ARW/threads:32/process_time/real_time_mean -0.0130 -0.0139 19 19 609 600 ./Sony/ILCE-7RM5/7RM5-S35-LosslessCompressedMedium.ARW/threads:32/process_time/real_time_median -0.0187 -0.0193 19 19 607 595 ./Sony/ILCE-7RM5/7RM5-S35-LosslessCompressedMedium.ARW/threads:32/process_time/real_time_stddev +0.2361 +0.1950 0 1 15 18 ./Sony/ILCE-7RM5/7RM5-S35-LosslessCompressedMedium.ARW/threads:32/process_time/real_time_cv +0.2524 +0.2118 0 0 0 0 OVERALL_GEOMEAN -0.0049 -0.0058 0 0 1 1 ``` ... and apparently what we were doing wasn't needed for performance either. Well, not particularly, at least.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For predictor mode 1, which is the only one supported currently, the predictor for the next MCU is the fully coded previous MCU. When going to the next row, we use the first MCU of the first row, or the initial predictors for the initial row.
As an effect, we can either
(a) keep decoded MCU (as new predictor) after decoding and storing it, or (b) load the previous MCU as the new predictor when needed.
We were doing (a), but that won't work for other predictor modes.
... and apparently what we were doing wasn't needed for performance either. Well, not particularly, at least.