Fails to build with gcc 14

[plorenzo]

Hi,

I know you are aware of the problem and it was already discussed
in the devel mailing list, just need an issue to link to the
Debian Bug tracking system.
Also, this is now considered a blocker for the next Debian release so if it isn't fixed around December2024/Gen2025, mplayer will likely miss the next Debian stable release.

links:

Debian bug #1075294

​https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1075294

patch on mplayer-dev

​https://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2024-April/074173.html

I haven't yet tested the patch, what's the risk if I use that to temporary build the deb package, new bugs or also security holes?

Thanks,
Lorenzo

[beastd]

Hi Lorenzo,

thanks for opening this ticket.

I'm optimistic we can get MPlayer to compile with GCC14.

As I'm really low on time at least for a month from now, it will probably take about 2 months minimum to fix. Maybe more realistically 3 months with review, rework, commits...

TL;DR: Would be good if you give the patch a chance. Maybe there is more missing in regard to the Debian builds and their configuration. I would be glad to hear about it should it work or not! I would not recommend to distribute the results though.

plorenzo wrote:

I haven't yet tested the patch, what's the risk if I use that to temporary build the deb package, new bugs or also security holes?

That's a bit hard to say. I see mainly 2 risks

1. The patch hasn't really seen much of review. Maybe some bad mistakes crept in by accident.
2. Everywhere we touch on undefined behavior (UB), there is a chance the compiler will do something wrong. Doesn't happen most of the time, but there were quite a few cases documented. Some with security impact.

Regarding risk 2 I hope the patch itself shouldn't really introduce newly undefined behavior that wasn't there previously. Just makes it more visible in the code and less visible in the compiler output because the warnings/errors are silenced.

So there is only the general risk that GCC14 will lay out the UB in ways that are less favorable for us or even dangerous.

[plorenzo]

Hi,
I tested a slightly refreshed version of the patch [1] with svn38638; builds fine
on amd64, fails on i386 with the following error

libmpcodecs/vd_qtvideo.c:132:20: error: assignment to 'char (*)(Size)' {aka 'char (*)(int)'} from incompatible pointer type 'OSErr (*)(Size)' {aka 'short int (*)(int)'} [-Wincompatible-pointer-types]

132 | NewHandleClear = (OSErr(*)(Size))GetProcAddress(handler, "NewHandleClear");

|

full build log in [2]
other than that, on amd64 mplayer seems to work fine (tested only few video files though)

Thanks,
Lorenzo

[1] ​https://salsa.debian.org/multimedia-team/mplayer/-/blob/next/debian/patches/0205-fix-FTBFS-gcc14.patch?ref_type=heads
[2] ​https://salsa.debian.org/multimedia-team/mplayer/-/jobs/6242311/raw

[beastd]

Thanks for trying the patch!

Unfortunately I was not able to continue work on this yet.

Hope I can make some time for it soon.

[beastd]

Since today most stuff is fixed starting with MPlayer SVN r38660 .

There are probably still some problems with x86 32bit and maybe on some other platforms.

Will try to look into the specific build problem you quoted above next.

[marillat@…]

I see this issue with r38660 Debian unstable amd64 gcc 14.2.0

In file included from libvo/vo_mga.c:52:
libvo/mga_template.c: In function 'draw_slice_g200':
libvo/mga_template.c:80:28: error: passing argument 2 of 'sws_scale' from incompatible pointer type [-Wincompatible-pointer-types]
   80 |         sws_scale(sws_ctx, image, stride, y, height, dst, dst_stride);
      |                            ^~~~~
      |                            |
      |                            uint8_t ** {aka unsigned char **}
In file included from libvo/mga_template.c:22:

[marillat@…]

I see this issue with r38660 Debian unstable i386 gcc 14.2.0
with mga driver disabled

libmpcodecs/ve_nuv.c: In function 'put_image':
libmpcodecs/ve_nuv.c:155:34: error: passing argument 4 of 'lzo1x_1_compress' from incompatible pointer type [-Wincompatible-pointer-types]
  155 |                            zdata,&zlen,vf->priv->zmem);
      |                                  ^~~~~
      |                                  |
      |                                  size_t * {aka unsigned int *}
In file included from libmpcodecs/ve_nuv.c:41:
/usr/include/lzo/lzo1x.h:75:58: note: expected 'lzo_uint *' {aka 'long unsigned int *'} but argument is of type 'size_t *' {aka 'unsigned int *'}
   75 |                                 lzo_bytep dst, lzo_uintp dst_len,
      |                                                          ^
libmpcodecs/ve_nuv.c:180:43: error: passing argument 4 of 'lzo1x_1_compress' from incompatible pointer type [-Wincompatible-pointer-types]
  180 |       r = lzo1x_1_compress(data,len,zdata,&zlen,vf->priv->zmem);
      |                                           ^~~~~
      |                                           |
      |                                           size_t * {aka unsigned int *}
/usr/include/lzo/lzo1x.h:75:58: note: expected 'lzo_uint *' {aka 'long unsigned int *'} but argument is of type 'size_t *' {aka 'unsigned int *'}
   75 |                                 lzo_bytep dst, lzo_uintp dst_len,
      |                                                          ^
make: *** [Makefile:729: libmpcodecs/ve_nuv.o] Error 1

[plorenzo]

Hi, quick update:
I've applied the patch from
​https://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2024-October/074231.html
and the error I reported for vd_qtvideo.c is fixed.
The build on i386 now fails with the same error in ve_nuv.c reported by Marillat

[ib]

Try:

--- a/libmpcodecs/ve_nuv.c
+++ b/libmpcodecs/ve_nuv.c
@@ -125,7 +125,7 @@
   uint8_t* data = vf->priv->buffer + FRAMEHEADERSIZE;
   uint8_t* zdata = vf->priv->zbuffer + FRAMEHEADERSIZE;
   int len = 0, r;
-  size_t zlen = 0;
+  lzo_uint zlen = 0;
 
   memset(header, 0, FRAMEHEADERSIZE); // Reset the header
   if(vf->priv->lzo)

[marillat@…]

Issue in comment 5 has been fixed with a rebuild of r38664
Proposed patch from plorenzo fix the build with 32 bits arches

But I see one issue with powerpc arch :

libmpcodecs/vf_scale.c: In function 'scale':
libmpcodecs/vf_scale.c:432:16: error: assignment to 'uint8_t *' {aka 'unsigned char *'} from incompatible pointer type 'uint32_t *' {aka 'unsigned int *'} [-Wincompatible-pointer-types]
  432 |         src2[1]= pal2;
      |                ^
libmpcodecs/vf_scale.c: At top level:
libmpcodecs/vf_scale.c:733:50: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  733 |   {"presize", 0, CONF_TYPE_OBJ_PRESETS, 0, 0, 0, &size_preset},
      |                                                  ^
make: *** [Makefile:729: libmpcodecs/vf_scale.o] Error 1

[plorenzo]

Hi Ingo, yes with your patch I can build on i386.
Thanks :)

[ib]

Replying to plorenzo:

with your patch I can build on i386.

Patch has been committed to svn.

[marillat@…]

The problem for powerpc come from commit 38647 "explicit pointer casts"

Index: libmpcodecs/vf_scale.c
===================================================================
--- libmpcodecs/vf_scale.c	(révision 38201)
+++ libmpcodecs/vf_scale.c	(révision 38647)
@@ -439,14 +439,14 @@
         int src_stride2[MP_MAX_PLANES]={2*src_stride[0], 2*src_stride[1], 2*src_stride[2], 2*src_stride[3]};
         int dst_stride2[MP_MAX_PLANES]={2*dst_stride[0], 2*dst_stride[1], 2*dst_stride[2], 2*dst_stride[3]};
 
-        sws_scale(sws1, src2, src_stride2, y>>1, h>>1, dst2, dst_stride2);
+        sws_scale(sws1, (const uint8_t *const *)src2, src_stride2, y>>1, h>>1, dst2, dst_stride2);
         for(i=0; i<MP_MAX_PLANES; i++){
             src2[i] += src_stride[i];
             dst2[i] += dst_stride[i];
         }
-        sws_scale(sws2, src2, src_stride2, y>>1, h>>1, dst2, dst_stride2);
+        sws_scale(sws2, (const uint8_t *const *)src2, src_stride2, y>>1, h>>1, dst2, dst_stride2);
     }else{
-        sws_scale(sws1, src2, src_stride, y, h, dst, dst_stride);
+        sws_scale(sws1, (const uint8_t *const *)src2, src_stride, y, h, dst, dst_stride);
     }
 

But as powerpc is bigendian the code above these changes have not been modified.

#if HAVE_BIGENDIAN
    uint32_t pal2[256];
    if (src[1] && !src[2]){
        int i;
        for(i=0; i<256; i++)
            pal2[i]= bswap_32(((uint32_t*)src[1])[i]);
        src2[1]= pal2;
    }
#endif