Add MinGW32 CI. #1887
Add MinGW32 CI. #1887
Conversation
|
FYI: https://www.msys2.org/news/#2023-12-13-starting-to-drop-some-32-bit-packages Will still be around and useful for CI for while longer I guess... |
|
Thx @kmilos for the info! Please open up bugs or PRs if there is anything else required on the MinGW front. |
| @@ -52,7 +53,7 @@ TEST(TransferCharacteristicsTest, RoundTrip) { | |||
| } | |||
|
|
|||
| if (tc == AVIF_TRANSFER_CHARACTERISTICS_LOG100) { | |||
| EXPECT_EQ(min_linear, kTransferLog100Threshold / 2.0f); | |||
| EXPECT_NEAR(min_linear, kTransferLog100Threshold / 2.0f, 1e-7); | |||
There was a problem hiding this comment.
Nit: The constants in line 56 and line 109 can have the f suffix.
There was a problem hiding this comment.
That matters because it is all about having bad float computations here... Done in #1888
| @@ -13,7 +13,8 @@ namespace { | |||
| constexpr int kMaxTransferCharacteristic = 18; | |||
|
|
|||
| // Thresholds in the transfer curve formulas. | |||
| constexpr float kTransferLog100Threshold = 0.01f; | |||
| // Add a little tolerance for test to pass on MinGW32. | |||
There was a problem hiding this comment.
It would be good to understand why we get different results on MinGW32. It seems that this test only does some float arithmetic and calls the powf() function. So it should get the same results on all platforms. Does this mean the powf() function on MinGW32 may return slightly different values?
There was a problem hiding this comment.
Indeed. So I copied the code over in the test and the values pass ... So is there a difference between the C and the C++ implementation ?? The main problem seems to also come from the constexpr because doing a dynamic comparison works ... I spent too much time on this already so just a little tolerance is fine :)
It is good to have this CI as this is the only one we have on 32 bit.