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

Implement binary for accuracy testing #16

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

FreezyLemon
Copy link
Contributor

@FreezyLemon FreezyLemon commented Mar 24, 2023

I had the idea a while ago, but never really got around to implementing it. I'm PRing in the current state to get some feedback, as I'm not 100% sure on where to take this and how to make it more useful.

One of the biggest questions is how to properly test the algorithm accuracy. So far, I've tried converting the source (sRGB) image with imagemagick via convert pic_srgb.png -colorspace RGB pic_rgb.png. But when comparing the converted image to a LinearRgb conversion (current code), you'll see that the error is relatively large (and that even turning fastmath off won't help much). Not sure what's going on there.

Run with cargo run --features build-acctest --bin acctest -r

@Luni-4
Copy link
Member

Luni-4 commented Jan 30, 2024

What is the status of this PR?

@FreezyLemon
Copy link
Contributor Author

At a glance:

  • comparison image paths are hardcoded, the binary should probably handle CLI parameters
  • At the moment, the expected output images are created outside the Rust code by external tools. This should be integrated
  • It should be checked/verified that the imagemagick conversions are accurate (or at least more accurate than ours). Maybe need to change to another tool to create the images we compare against.

@Luni-4
Copy link
Member

Luni-4 commented Jan 30, 2024

* comparison image paths are hardcoded, the binary should probably handle CLI parameters

I agree about the CLI parameters for image paths

* At the moment, the expected output images are created outside the Rust code by external tools. This should be integrated

I also agree with this change

* It should be checked/verified that the imagemagick conversions are accurate (or at least more accurate than ours). Maybe need to change to another tool to create the images we compare against.

As intermediate goal, we can produce images which are equals to the one created with imagemagick and then evaluate if the process used in creating those images is correct or not. I think it would be better having everything in the same tool initially and split it up eventually

@FreezyLemon
Copy link
Contributor Author

As intermediate goal, we can produce images which are equals to the one created with imagemagick and then evaluate if the process used in creating those images is correct or not. I think it would be better having everything in the same tool initially and split it up eventually

I am not sure if I understand you correctly. The original goal was to

  1. Take an input image
  2. Convert it with yuvxyb
  3. Compare the converted image to a reference
  4. Visualize the relative differences between the two

The big question is: What to use as a reference? I used imagemagick to create the references for this draft, but it seems to have a significant delta to our conversions. This could indicate three things in my mind:

  • I made a mistake when creating the reference images,
  • the yuvxyb conversion is bad, or
  • imagemagick's conversion is less accurate than expected

I don't really know enough about imagemagick (or possible alternatives) to solve this without some significant time investment.

@Luni-4
Copy link
Member

Luni-4 commented Jan 30, 2024

Ok now I better see the problem, thanks!

Can we use the test card as minimal case in order to isolate where deltas are?

For example, if we have 3 frameworks, we can pass the test card to these 3 frameworks as input obtaining 3 output images. If these output images are all equals to each other, then it means our algorithm is wrong. If there are differences among them, we can discover what are these differences, and analyzing their algorithms, we can discover something more.

I do not know if this procedure is feasible or if I've figured out correctly the issue.

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.

2 participants