Bug 2261310 - libdv: FTBFS in Fedora rawhide/f40
Summary: libdv: FTBFS in Fedora rawhide/f40
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: libdv
Version: rawhide
Hardware: i686
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Antonio T. sagitter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: F40FTBFS
TreeView+ depends on / blocked
 
Reported: 2024-01-29 20:30 UTC by Fedora Release Engineering
Modified: 2024-03-17 21:41 UTC (History)
4 users (show)

Fixed In Version: libdv-1.0.0-42.fc41
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2024-03-17 21:41:17 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
build.log (32.00 KB, text/plain)
2024-01-29 20:30 UTC, Fedora Release Engineering
no flags Details
root.log (32.00 KB, text/plain)
2024-01-29 20:30 UTC, Fedora Release Engineering
no flags Details
state.log (1.62 KB, text/plain)
2024-01-29 20:30 UTC, Fedora Release Engineering
no flags Details
Clear the offsetof warnings (643 bytes, patch)
2024-02-25 07:59 UTC, Jeffrey Walton
no flags Details | Diff
Proposed patch for 2261310 on i686 (1.05 KB, patch)
2024-02-25 21:55 UTC, Jeffrey Walton
no flags Details | Diff

Description Fedora Release Engineering 2024-01-29 20:30:16 UTC
libdv failed to build from source in Fedora rawhide/f40

https://koji.fedoraproject.org/koji/taskinfo?taskID=112313165


For details on the mass rebuild see:

https://fedoraproject.org/wiki/Fedora_40_Mass_Rebuild
Please fix libdv at your earliest convenience and set the bug's status to
ASSIGNED when you start fixing it. If the bug remains in NEW state for 8 weeks,
libdv will be orphaned. Before branching of Fedora 41,
libdv will be retired, if it still fails to build.

For more details on the FTBFS policy, please visit:
https://docs.fedoraproject.org/en-US/fesco/Fails_to_build_from_source_Fails_to_install/

Comment 1 Fedora Release Engineering 2024-01-29 20:30:22 UTC
Created attachment 2012430 [details]
build.log

file build.log too big, will only attach last 32768 bytes

Comment 2 Fedora Release Engineering 2024-01-29 20:30:27 UTC
Created attachment 2012431 [details]
root.log

file root.log too big, will only attach last 32768 bytes

Comment 3 Fedora Release Engineering 2024-01-29 20:30:30 UTC
Created attachment 2012432 [details]
state.log

Comment 4 Jeffrey Walton 2024-02-25 07:18:45 UTC
Autoreconf is failing for this package:

configure.ac:54: error: possibly undefined macro: AC_DEFINE
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
configure.ac:179: error: possibly undefined macro: AM_PATH_SDL
autoreconf: error: /usr/bin/autoconf failed with exit status: 1

I think the libdv.spec file needs to include a buildrequires for SDL.

Comment 5 Jeffrey Walton 2024-02-25 07:58:36 UTC
I can't reproduce the issue using the latest Rawhide packages.

You can probably close this issue.

Comment 6 Jeffrey Walton 2024-02-25 07:59:21 UTC
Created attachment 2018621 [details]
Clear the offsetof warnings

Comment 7 Antonio T. sagitter 2024-02-25 16:53:17 UTC
Hi Jeffrey.

Unfortunately, it's failing on i686 yet:

/bin/sh ../libtool --silent  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I..     -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m32 -march=i686 -mtune=generic -msse2 -mfpmath=sse -mstackrealign -fasynchronous-unwind-tables -fstack-clash-protection  -Wall -g -c -o dv.lo dv.c
make[3]: Leaving directory '/builddir/build/BUILD/libdv-1.0.0/libdv'
dv.c: In function 'dv_decode_macroblock':
dv.c:224:90: error: passing argument 5 of '_dv_quant_88_inverse_x86' from incompatible pointer type [-Wincompatible-pointer-types]
  224 |       _dv_quant_88_inverse_x86(mb->b[i].coeffs,mb->qno,mb->b[i].class_no,dv_quant_offset,dv_quant_shifts);
      |                                                                                          ^~~~~~~~~~~~~~~
      |                                                                                          |
      |                                                                                          uint8_t (*)[4] {aka unsigned char (*)[4]}
In file included from dv.c:50:
quant.h:30:101: note: expected 'uint8_t *' {aka 'unsigned char *'} but argument is of type 'uint8_t (*)[4]' {aka 'unsigned char (*)[4]'}
   30 | extern void _dv_quant_88_inverse_x86(dv_coeff_t *block,int qno,int klass, uint8_t *offset, uint8_t *shifts);
      |                                                                                            ~~~~~~~~~^~~~~~
dv.c: In function 'dv_decode_video_segment':
dv.c:256:82: error: passing argument 5 of '_dv_quant_88_inverse_x86' from incompatible pointer type [-Wincompatible-pointer-types]
  256 |         _dv_quant_88_inverse_x86(bl->coeffs,mb->qno,bl->class_no,dv_quant_offset,dv_quant_shifts);
      |                                                                                  ^~~~~~~~~~~~~~~
      |                                                                                  |
      |                                                                                  uint8_t (*)[4] {aka unsigned char (*)[4]}
quant.h:30:101: note: expected 'uint8_t *' {aka 'unsigned char *'} but argument is of type 'uint8_t (*)[4]' {aka 'unsigned char (*)[4]'}
   30 | extern void _dv_quant_88_inverse_x86(dv_coeff_t *block,int qno,int klass, uint8_t *offset, uint8_t *shifts);
      |                                                                                            ~~~~~~~~~^~~~~~
dv.c: In function 'dv_report_video_error':
dv.c:604:10: warning: variable 'error_code' set but not used [-Wunused-but-set-variable]
  604 |   int i, error_code;
      |          ^~~~~~~~~~
make[3]: *** [Makefile:691: dv.lo] Error 1

Comment 8 Jeffrey Walton 2024-02-25 17:20:20 UTC
Oh, thanks Antonio. I did not notice it was an i686 problem. I was testing from x86_64.

I _thought_ Fedora dropped i686 support. Cf., <https://fedoraproject.org/wiki/Changes/EncourageI686LeafRemoval>.

How does one test i686?

(I still support 32-bit platforms in my software, so I don't mind helping out. I just need to know how to reproduce the issue).

Comment 9 Antonio T. sagitter 2024-02-25 19:44:18 UTC
> How does one test i686?

Use 'mock' for a local rebuild:

$ wget https://kojipkgs.fedoraproject.org//packages/libdv/1.0.0/39.fc39/src/libdv-1.0.0-39.fc39.src.rpm
$ mock -r fedora-rawhide-i386 rebuild libdv-1.0.0-39.fc39.src.rpm

Comment 10 Jeffrey Walton 2024-02-25 21:55:21 UTC
Thanks again Antonio.

So looking at `dv.c`, I see:

```
// line 208 below
extern uint8_t dv_quant_offset[4];
extern uint8_t dv_quant_shifts[22][4];
...

// line 213 below
  int i;
  for (i=0;
       i<((quality & DV_QUALITY_COLOR) ? 6 : 4);
       i++) {
 
// line 223 below
#if ARCH_X86
      _dv_quant_88_inverse_x86(mb->b[i].coeffs,mb->qno,mb->b[i].class_no,dv_quant_offset,dv_quant_shifts);
      _dv_idct_88(mb->b[i].coeffs);
#elif ARCH_X86_64
      _dv_quant_88_inverse_x86_64(mb->b[i].coeffs,mb->qno,mb->b[i].class_no);
      _dv_idct_88(mb->b[i].coeffs);
#else /* ARCH_X86 */
      _dv_quant_88_inverse(mb->b[i].coeffs,mb->qno,mb->b[i].class_no);
      _dv_weight_88_inverse(mb->b[i].coeffs);
      _dv_idct_88(mb->b[i].coeffs);
#endif /* ARCH_X86 */

  ...
} // end for loop
```

I _think_ the fix is either:

1. Change `dv_quant_shifts` to `dv_quant_shifts[i]`:

```
_dv_quant_88_inverse_x86(mb->b[i].coeffs,mb->qno,mb->b[i].class_no,dv_quant_offset,dv_quant_shifts[i]);
```

2. Change `dv_quant_shifts` to `dv_quant_shifts[0]`:
```
_dv_quant_88_inverse_x86(mb->b[i].coeffs,mb->qno,mb->b[i].class_no,dv_quant_offset,dv_quant_shifts[0]);
```

I don't believe (1) is correct in this context since there are 22 rows for `dv_quant_shifts`, but the loop is only running to 4 or 6. So if we believe this is the case, then 22-4=18 (or 22-6=16) rows are unused. It smells a bit off.

I believe the answer is (2) because array names decay to pointers, so dv_quant_shifts[22][4] effectively becomes `&dv_quant_shifts[0][0]` once the array decays, which we can write as `dv_quant_shifts[0]` in code.

We have preserved previous behavior, but it could still be wrong. I don't know how to make the change and test the fix (so that it is persistent for mock). But I attached the proposed patch anyways.

Comment 11 Jeffrey Walton 2024-02-25 21:55:56 UTC
Created attachment 2018823 [details]
Proposed patch for 2261310 on i686

Comment 12 Jeffrey Walton 2024-02-25 22:37:49 UTC
Antonio,

One last comment. This compiles as expected. The first call to foo() produces the warning; the second one does not.

```
$ cat t.c
#include <stdint.h>

extern void bar (uint8_t* params);

uint8_t dv_quant_shifts[22][4];

void foo (uint8_t params[4])
{
    bar (params);
}

int main (int arcg, char* argv[])
{
    // Passing argument 1 of ‘foo’ from incompatible pointer type
    foo (dv_quant_shifts);
    // Ok
    foo (dv_quant_shifts[0]);
    
    return 0;
}
```

And notice the warning only shows for the first call to foo():

$ gcc -c t.c -o t.o
t.c: In function ‘main’:
t.c:15:10: warning: passing argument 1 of ‘foo’ from incompatible pointer type [-Wincompatible-pointer-types]
   15 |     foo (dv_quant_shifts);
      |          ^~~~~~~~~~~~~~~
      |          |
      |          uint8_t (*)[4] {aka unsigned char (*)[4]}
t.c:7:19: note: expected ‘uint8_t *’ {aka ‘unsigned char *’} but argument is of type ‘uint8_t (*)[4]’ {aka ‘unsigned char (*)[4]’}
    7 | void foo (uint8_t params[4])
      |           ~~~~~~~~^~~~~~~~~

Comment 13 Antonio T. sagitter 2024-02-27 18:03:28 UTC
Thanks for your precious help.
These errors come out with your changes.

(In reply to Jeffrey Walton from comment #11)
> Created attachment 2018823 [details]
> Proposed patch for 2261310 on i686

```
dv.c: In function 'dv_decode_macroblock':
dv.c:224:105: error: passing argument 5 of '_dv_quant_88_inverse_x86' from incompatible pointer type [-Wincompatible-pointer-types]
  224 |       _dv_quant_88_inverse_x86(mb->b[i].coeffs,mb->qno,mb->b[i].class_no,dv_quant_offset,dv_quant_shifts[0]);
      |                                                                                          ~~~~~~~~~~~~~~~^~~
      |                                                                                                         |
      |                                                                                                         uint8_t * {aka unsigned char *}
In file included from dv.c:50:
quant.h:30:102: note: expected 'uint8_t (*)[4]' {aka 'unsigned char (*)[4]'} but argument is of type 'uint8_t *' {aka 'unsigned char *'}
   30 | extern void _dv_quant_88_inverse_x86(dv_coeff_t *block,int qno,int klass, uint8_t *offset, uint8_t (*shifts)[4]);
      |                                                                                            ~~~~~~~~~~^~~~~~~~~~
dv.c: In function 'dv_decode_video_segment':
dv.c:256:97: error: passing argument 5 of '_dv_quant_88_inverse_x86' from incompatible pointer type [-Wincompatible-pointer-types]
  256 |         _dv_quant_88_inverse_x86(bl->coeffs,mb->qno,bl->class_no,dv_quant_offset,dv_quant_shifts[0]);
      |                                                                                  ~~~~~~~~~~~~~~~^~~
      |                                                                                                 |
      |                                                                                                 uint8_t * {aka unsigned char *}
quant.h:30:102: note: expected 'uint8_t (*)[4]' {aka 'unsigned char (*)[4]'} but argument is of type 'uint8_t *' {aka 'unsigned char *'}
   30 | extern void _dv_quant_88_inverse_x86(dv_coeff_t *block,int qno,int klass, uint8_t *offset, uint8_t (*shifts)[4]);
      |                                                                                            ~~~~~~~~~~^~~~~~~~~~
dv.c: In function 'dv_report_video_error':
dv.c:604:10: warning: variable 'error_code' set but not used [-Wunused-but-set-variable]
  604 |   int i, error_code;
      |          ^~~~~~~~~~
make[3]: *** [Makefile:691: dv.lo] Error 1
```

And

```
make[3]: Leaving directory '/builddir/build/BUILD/libdv-1.0.0/libdv'
quant.c: In function '_dv_quant':
quant.c:213:67: error: passing argument 5 of '_dv_quant_x86' from incompatible pointer type [-Wincompatible-pointer-types]
  213 |                 _dv_quant_x86(block, qno, klass, dv_quant_offset, dv_quant_shifts);
      |                                                                   ^~~~~~~~~~~~~~~
      |                                                                   |
      |                                                                   uint8_t (*)[4] {aka unsigned char (*)[4]}
quant.c:147:109: note: expected 'uint8_t *' {aka 'unsigned char *'} but argument is of type 'uint8_t (*)[4]' {aka 'unsigned char (*)[4]'}
  147 | extern void             _dv_quant_x86(dv_coeff_t *block,int qno,int klass,uint8_t *dv_quant_offset,uint8_t *dv_quant_shifts);
      |                                                                                                    ~~~~~~~~~^~~~~~~~~~~~~~~
make[3]: *** [Makefile:691: quant.lo] Error 1
```

Comment 14 Fedora Update System 2024-03-17 21:36:35 UTC
FEDORA-2024-33233d43b9 (libdv-1.0.0-42.fc41) has been submitted as an update to Fedora 41.
https://bodhi.fedoraproject.org/updates/FEDORA-2024-33233d43b9

Comment 15 Fedora Update System 2024-03-17 21:41:17 UTC
FEDORA-2024-33233d43b9 (libdv-1.0.0-42.fc41) has been pushed to the Fedora 41 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.