Skip to content

Handle images with odd dimensions and fix color matrix, and add CI to check decoding functions #9

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

Merged

Conversation

ledyba-z
Copy link
Collaborator

@ledyba-z ledyba-z commented Feb 4, 2020

Hi!

I found some bugs, and fixed:

  • It will crash or display an inaccurate image when the decoder reads images with odd-width or odd-height.
  • Conversion matrix is wrong (matrix->Cr_G and matrix->Cb_G are swapped).

And also, I added automated tests using github actions, because it is very boring and hard to check all cases by hand.

It checks that:

  • the decoder won't crash when it reads all images in our sample images
  • Output images are not so different from original (AVIF is lossy format, so the output image will not exactly match the original image).

Result page is here:
https://github.com/link-u/SDWebImageAVIFCoder/actions/runs/34667715

If travis-ci is more appropriate, I will port it.

Please take a look!

@ledyba-z ledyba-z requested a review from dreampiggy February 4, 2020 15:25
@dreampiggy
Copy link
Collaborator

Hi. Nice to see your contribution again.

I’ll have a check tomorrow, the CI integration is good (Previously, because of lack of unit test, I just use the Example samples).

A CLI for macOS is OK, we can also use the same test cases like SDWebImage or SDWebImageWebPCoder as well, which is much better for Xcode and can collect code coverage.

Copy link
Collaborator

@dreampiggy dreampiggy left a comment

Choose a reason for hiding this comment

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

The code change part LGTM +1.

The CI runner part, what else should I do to turn on GitHub Actions ? I previouslly only use the Travis-CI to build Unit Test.

@dreampiggy
Copy link
Collaborator

Let me just merge and have a try ?

@ledyba-z
Copy link
Collaborator Author

ledyba-z commented Feb 7, 2020

@dreampiggy

Thank you for your review! (and sorry for the late reply...)

we can also use the same test cases like SDWebImage or SDWebImageWebPCoder as well, which is much better for Xcode and can collect code coverage.

I agree. If you need, I will create some additional test images (which are more simple to write unit tests).

The CI runner part, what else should I do to turn on GitHub Actions ?

Just put yaml files to .github/workflows/****.yml.

Let me just merge and have a try ?

Of course! After this patch will be merged, Github Actions will be turned on.

@dreampiggy dreampiggy merged commit f8570a2 into SDWebImage:master Feb 7, 2020
@ledyba-z ledyba-z deleted the fix/odd-dimensions-and-color-matrix branch February 14, 2020 02:45
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