Bug 216537

Summary: graphics/sekrit-twc-zimg fails ColorspaceConversionTest.test_transfer_only test
Product: Ports & Packages Reporter: Peter Kien <peter.kien>
Component: Individual Port(s)Assignee: Jan Beich <jbeich>
Status: Closed FIXED    
Severity: Affects Some People CC: mi, peter.kien, w.schwarzenfeld
Priority: --- Flags: jbeich: maintainer-feedback+
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
test-suite.log none

Description Peter Kien 2017-01-27 23:09:55 UTC
Hello :)

The "testsuite summary for zimg 2.4.0" states the following:

---------------------------------------------------------------
[==========] 93 tests from 22 test cases ran. (28927 ms total)
[  PASSED  ] 92 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ColorspaceConversionTest.test_transfer_only
---------------------------------------------------------------

This also happens when "SIMD" is set to OFF and the make.conf only
contains "DEFAULT_VERSIONS+=ssl=libressl", which shouldn't influence
the issue.

Thanks for looking into it.
Comment 1 Walter Schwarzenfeld freebsd_triage 2018-01-16 09:56:12 UTC
We have version at 2.7.1. Is this stlil relevant?
Comment 2 Jan Beich freebsd_committer freebsd_triage 2018-01-22 05:09:35 UTC
As of 2.7.1 some tests still fail. I don't have time to investigate.

11.1 amd64: https://clbin.com/4cFJT
[==========] 119 tests from 24 test cases ran. (26227 ms total)
[  PASSED  ] 117 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] ColorspaceConversionTest.test_transfer_only_b67
[  FAILED  ] ColorspaceConversionTest.test_rec2100_ictcp

11.1 i386: https://clbin.com/YYNGr
[==========] 119 tests from 24 test cases ran. (28169 ms total)
[  PASSED  ] 104 tests.
[  FAILED  ] 15 tests, listed below:
[  FAILED  ] ResizeImplSSETest.test_resize_h_f32
[  FAILED  ] ResizeImplAVX2Test.test_resize_h_f32
[  FAILED  ] ResizeImplAVXTest.test_resize_h_f32
[  FAILED  ] ColorspaceConversionSSE2Test.test_transfer_lut
[  FAILED  ] ColorspaceConversionAVX2Test.test_transfer_lut
[  FAILED  ] ResizeImplTest.test_horizontal_down
[  FAILED  ] ResizeImplTest.test_vertical_down
[  FAILED  ] ResizeImplTest.test_horizontal_shift
[  FAILED  ] ResizeImplTest.test_vertical_shift
[  FAILED  ] ColorspaceConversionTest.test_transfer_only
[  FAILED  ] ColorspaceConversionTest.test_transfer_only_b67
[  FAILED  ] ColorspaceConversionTest.test_matrix_transfer
[  FAILED  ] ColorspaceConversionTest.test_matrix_transfer_primaries
[  FAILED  ] ColorspaceConversionTest.test_constant_luminance
[  FAILED  ] ColorspaceConversionTest.test_rec2100_ictcp
Comment 3 Jan Beich freebsd_committer freebsd_triage 2018-01-22 05:47:09 UTC
On FreeBSD 10.* unit_test crashes before completing. Affects Clang/libc++ but not GCC/libstdc++.

[----------] 8 tests from ErrorDiffusionAVX2Test
[ RUN      ] ErrorDiffusionAVX2Test.test_error_diffusion_b2b
[       OK ] ErrorDiffusionAVX2Test.test_error_diffusion_b2b (201 ms)
[ RUN      ] ErrorDiffusionAVX2Test.test_error_diffusion_b2w
[       OK ] ErrorDiffusionAVX2Test.test_error_diffusion_b2w (203 ms)
[ RUN      ] ErrorDiffusionAVX2Test.test_error_diffusion_w2b

Program received signal SIGBUS, Bus error.
0x0000000000613b29 in zimg::depth::(anonymous namespace)::error_diffusion_traits<(zimg::PixelType)1>::load8 (ptr=0x8024385dc) at src/zimg/depth/x86/error_diffusion_avx2.cpp:68
68              {
(gdb) list
63
64              static float load1(const uint16_t *ptr) { return *ptr; }
65              static void store1(uint16_t *ptr, uint32_t x) { *ptr = static_cast<uint32_t>(x); }
66
67              static __m256 load8(const uint16_t *ptr)
68              {
69                      return _mm256_cvtepi32_ps(_mm256_cvtepu16_epi32(_mm_load_si128((const __m128i *)ptr)));
70              }
71
72              static void store8(uint16_t *ptr, __m256i x)
(gdb) backtrace
#0  0x0000000000613b29 in zimg::depth::(anonymous namespace)::error_diffusion_traits<(zimg::PixelType)1>::load8 (ptr=0x8024385dc) at src/zimg/depth/x86/error_diffusion_avx2.cpp:68
#1  0x00000000006143bb in zimg::depth::(anonymous namespace)::error_diffusion_wf_avx2<(zimg::PixelType)1, (zimg::PixelType)0, unsigned short, unsigned char> (src=..., dst=..., i=0,
    error_top=0x802539a08, error_cur=0x802539000, state=0x7fffffffd040, scale=0.00390625, offset=0,
    bits=8, width=624) at src/zimg/depth/x86/error_diffusion_avx2.cpp:258
#2  0x00000000005f7635 in zimg::depth::(anonymous namespace)::error_diffusion_avx2<(zimg::PixelType)1, (zimg::PixelType)0> (src=..., dst=..., i=0, error_top=0x802539a08, error_cur=0x802539000,
    scale=0.00390625, offset=0, bits=8, width=640)
    at src/zimg/depth/x86/error_diffusion_avx2.cpp:370
#3  0x00000000005f4f1e in zimg::depth::(anonymous namespace)::ErrorDiffusionAVX2::process_vector (
    this=0x8024163c0, ctx=0x802539000, src=..., dst=..., i=0)
    at src/zimg/depth/x86/error_diffusion_avx2.cpp:475
#4  0x00000000005f4d25 in zimg::depth::(anonymous namespace)::ErrorDiffusionAVX2::process (
    this=0x8024163c0, ctx=0x802539000, src=0x7fffffffd3e0, dst=0x7fffffffd398, i=0)
    at src/zimg/depth/x86/error_diffusion_avx2.cpp:557
#5  0x000000000043884a in (anonymous namespace)::validate_filter_plane<unsigned short, unsigned char> (filter=0x8024163c0, src_buffer=0x7fffffffd860, dst_buffer=0x7fffffffd798)
    at test/graph/filter_validator.cpp:191
#6  0x000000000043ce23 in (anonymous namespace)::ValidateFilter<unsigned short, unsigned char>::operator() (this=0x7fffffffdae8, filter=0x8024163c0, src_width=640, src_height=480, src_format=...,
    yuv=false, sha1_str=0x7fffffffe2e0) at test/graph/filter_validator.cpp:271
#7  0x0000000000431199 in (anonymous namespace)::dispatch<(anonymous namespace)::ValidateFilter, zimg::graph::ImageFilter const*&, unsigned int&, unsigned int&, zimg::PixelFormat&, bool&, char const* const*&> (src_type=zimg::PixelType::WORD, dst_type=zimg::PixelType::BYTE,
    args=@0x7fffffffdfb8: 0x7fffffffe2e0, args=@0x7fffffffdfb8: 0x7fffffffe2e0,
    args=@0x7fffffffdfb8: 0x7fffffffe2e0, args=@0x7fffffffdfb8: 0x7fffffffe2e0,
    args=@0x7fffffffdfb8: 0x7fffffffe2e0, args=@0x7fffffffdfb8: 0x7fffffffe2e0)
    at test/graph/filter_validator.cpp:329
#8  0x00000000004306b0 in FilterValidator::validate (this=0x7fffffffdf90)
    at test/graph/filter_validator.cpp:385
#9  0x0000000000469860 in (anonymous namespace)::test_case (pixel_in=..., pixel_out=...,
    expected_sha1=0x7fffffffe2e0, expected_snr=inf)
    at test/depth/x86/error_diffusion_avx2_test.cpp:32
#10 0x000000000046a0a9 in ErrorDiffusionAVX2Test_test_error_diffusion_w2b_Test::TestBody (
    this=0x8024180d0) at test/depth/x86/error_diffusion_avx2_test.cpp:74
#11 0x0000000800c3b2e3 in testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void> (object=0x8024180d0, method=&virtual testing::Test::TestBody(),
    location=0x800c5288d "the test body") at ./src/gtest.cc:2401
#12 0x0000000800c23617 in testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void> (object=0x8024180d0, method=&virtual testing::Test::TestBody(),
    location=0x800c5288d "the test body") at ./src/gtest.cc:2437
#13 0x0000000800bf95a5 in testing::Test::Run (this=0x8024180d0) at ./src/gtest.cc:2473
#14 0x0000000800bfa74b in testing::TestInfo::Run (this=0x80241d240) at ./src/gtest.cc:2651
#15 0x0000000800bfb547 in testing::TestCase::Run (this=0x80241d080) at ./src/gtest.cc:2769
#16 0x0000000800c086c0 in testing::internal::UnitTestImpl::RunAllTests (this=0x80241f1c0)
    at ./src/gtest.cc:4665
#17 0x0000000800c376c3 in testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> (object=0x80241f1c0,
    method=(bool (testing::internal::UnitTestImpl::*)(testing::internal::UnitTestImpl * const)) 0x800c082a0 <testing::internal::UnitTestImpl::RunAllTests()>,
    location=0x800c52fc7 "auxiliary test code (environments or event listeners)")
    at ./src/gtest.cc:2401
#18 0x0000000800c26257 in testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> (object=0x80241f1c0,
    method=(bool (testing::internal::UnitTestImpl::*)(testing::internal::UnitTestImpl * const)) 0x800c082a0 <testing::internal::UnitTestImpl::RunAllTests()>,
    location=0x800c52fc7 "auxiliary test code (environments or event listeners)")
    at ./src/gtest.cc:2437
#19 0x0000000800c08219 in testing::UnitTest::Run (
    this=0x800e6dc58 <testing::UnitTest::GetInstance()::instance>) at ./src/gtest.cc:4277
#20 0x0000000000406891 in RUN_ALL_TESTS () at /usr/local/include/gtest/gtest.h:2231
#21 0x000000000040683b in main (argc=1, argv=0x7fffffffeb68) at test/main.cpp:35
Comment 4 Mikhail Teterin freebsd_committer freebsd_triage 2018-12-28 20:00:06 UTC
Created attachment 200589 [details]
test-suite.log

Still seeing these errors a year later -- on 11.2 stable.
Comment 5 Walter Schwarzenfeld freebsd_triage 2019-08-08 13:36:37 UTC
Failed also with version 2.9.2.
Comment 6 commit-hook freebsd_committer freebsd_triage 2019-10-05 00:42:50 UTC
A commit references this bug:

Author: jbeich
Date: Sat Oct  5 00:42:04 UTC 2019
New revision: 513800
URL: https://svnweb.freebsd.org/changeset/ports/513800

Log:
  graphics/sekrit-twc-zimg: use bundled math library to unbreak some tests

  [==========] 129 tests from 25 test suites ran. (29301 ms total)
  [  PASSED  ] 127 tests.
  [  FAILED  ] 2 tests, listed below:
  [  FAILED  ] ColorspaceConversionTest.test_transfer_only_b67
  [  FAILED  ] ColorspaceConversionTest.test_rec2100_ictcp

  PR:		216537

Changes:
  head/graphics/sekrit-twc-zimg/Makefile
  head/graphics/sekrit-twc-zimg/files/patch-no-extra-deps
Comment 7 Jan Beich freebsd_committer freebsd_triage 2019-10-05 00:58:15 UTC
Likely accuracy of some functions differed. Affected only tests.
Comment 8 Mikhail Teterin freebsd_committer freebsd_triage 2019-10-06 23:55:59 UTC
The fix is rather unsatisfying -- either our math library is broken, or the author's expectations are invalid.

In either case, using the bundled implementation is wrong... See also:

https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/bundled-libs.html
Comment 9 Jan Beich freebsd_committer freebsd_triage 2019-10-07 14:02:18 UTC
(In reply to Mikhail Teterin from comment #8)
> either our math library is broken, or the author's expectations are invalid.

Why do you think upstream uses musl math instead of system even on Linux? Accuracy (unlike precision specified by return types) differs between implementations. Given tests use SHA1 fingerprints a neglible difference can produce false positives.

(In reply to Mikhail Teterin from comment #8)
> In either case, using the bundled implementation is wrong...

musl math is only used by tests. Unbundling only makes sense when it doesn't introduce disproportional maintenance burden.
Comment 10 Mikhail Teterin freebsd_committer freebsd_triage 2019-10-07 16:36:36 UTC
(In reply to Jan Beich from comment #9)
> Accuracy (unlike precision specified by return types) differs
> between implementations

If the author expects higher accuracy than our library(ies) provide, then either his expectations are invalid -- and need adjusting -- or our libraries are buggy.

That's all...