Undeclared constant 'M_PI' in xvid_vbr macro 'DEG2RAD'

[bug]

Summary of the bug:
Undeclared constant 'M_PI' in xvid_vbr macro 'DEG2RAD'

How to reproduce:
Compiling mencoder in media-autobuild suite (MSYS2/MinGW, GCC 13.2)

xvid_vbr.c: In function 'vbr_init_2pass2':
xvid_vbr.c:44:18: error: 'M_PI' undeclared (first use in this function)
   44 | #define DEG2RAD (M_PI / 180.0)
      |                  ^~~~
xvid_vbr.c:887:53: note: in expansion of macro 'DEG2RAD'
  887 |                                         (1.0 +  sin(DEG2RAD * (state->average_frame * 90.0 / state->alt_curve_low_diff)));
      |                                                     ^~~~~~~
xvid_vbr.c:44:18: note: each undeclared identifier is reported only once for each function it appears in
   44 | #define DEG2RAD (M_PI / 180.0)
      |                  ^~~~
xvid_vbr.c:887:53: note: in expansion of macro 'DEG2RAD'
  887 |                                         (1.0 +  sin(DEG2RAD * (state->average_frame * 90.0 / state->alt_curve_low_diff)));
      |                                                     ^~~~~~~
xvid_vbr.c: In function 'vbr_getquant_2pass2':
xvid_vbr.c:44:18: error: 'M_PI' undeclared (first use in this function)
   44 | #define DEG2RAD (M_PI / 180.0)
      |                  ^~~~
xvid_vbr.c:1250:84: note: in expansion of macro 'DEG2RAD'
 1250 |                                                                                sin(DEG2RAD * ((dbytes - state->average_frame) * 90.0 / state->alt_curve_high_diff)));
      |                                                                                    ^~~~~~~

[beastd]

Thanks for the report!

Do you know what changed so this is a problem now?

For me it always worked so far. Unfortunately M_PI seems to be in not even a single C standard so far...

Does attachment:0001-xvid_vbr-Include-lavu-mathematics.h-to-make-sure-the.patch​ fix the problem for your builds?

[LigH]

The reason might be GCC 14.1 with stricter default warnings-as-errors than 13.2 before.

Patch applied; error moved to libaf/af_equalizer.c

[beastd]

Ah OK that might be related. GCC 14 will definitely need some more work...

I have put a patch on the mplayer development mailing list:

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

But it is not really the way to fix those problems, but if you need GCC 14 build, it should probably work in practice.

[LigH]

I may mention it in the M-AB-S issues, but will probably wait for your official commit and the closing of this ticket (there is only little need for mplayer/mencoder in these days). For now, thank you for your efforts.

[LigH]

So that was in April ... the mailing list contains a few steps of success but no final result, AFAICS.

And April is the last archived month of this mailing list. May 2024 does not even exist anymore.

[4Selur@…]

Any news on this?

[beastd]

Could you please try to build again with latest MPlayer development version (SVN r38666)?

Edit:
And possibly with attachment:0001-xvid_vbr-Include-lavu-mathematics.h-to-make-sure-the.patch​​ applied on top.

[LigH]

Building without patch: Same error.

Building with patch: Error moved.

[LigH]

Build and install logs of unpatched mplayer

[LigH]

Build and install logs of patched mplayer

[beastd]

Please give it a try with this new patch attachment:0001-Include-lavu-mathematics.h-everywhere-M_PI-is-used.patch​ I just attached.

[LigH]

On it. Will attach when done. The logs are huge now, many more warnings and errors...

[LigH]

Logs of latest patched build

[beastd]

@LigH: The latest errors look as if they are because of your LDFLAGS
-D_FORTIFY_SOURCE=2 -fstack-protector-strong -mtune=generic -O2 -pipe -D__USE_MINGW_ANSI_STDIO=1 -static-libgcc -static-libstdc++

Not really sure about this, but maybe try to add standard C++ lib to the LDFLAGS:
-D_FORTIFY_SOURCE=2 -fstack-protector-strong -mtune=generic -O2 -pipe -D__USE_MINGW_ANSI_STDIO=1 -static-libgcc -static-libstdc++ -lstdc++

Did statically linking standard C++ lib work before?

[LigH]

I am not really a developer, I only let the media-autobuild suite run and hope that it builds mplayer successfully; if not, I can interpret the errors as halfwit and ask more competent people for opinions.

I mentioned your suggestion in the ​M-AB-S issue 2702

[LigH]

I tried a lot (without any real clue) and was not successful yet...

​last results

[LigH]

​Solution in M-AB-S: Use gnu11 instead of c11 as C++ version in configure, and CXX instead of CC as compiler and linker

[LigH]

1 month later ... was it useful?

[beastd]

1 month later ... was it useful?

I think so!

Not yet entirely sure on how we fix in MPlayer sources, but I think I understand more of the background now...

With latest M-AB-S it worked for you, right?

Could you also try to build with my latest patch applied and without the grep_or_sed gnu11 configure 's/c11/gnu11/g' (in build/media-suite_compile.sh around line 2514) of the M-AB-S fix?

[LigH]

Nobody updated your repo yet, still at SVN rev. 38666

[LigH]

I possibly missed your "latest patch", do you have a link?

[beastd]

Sorry if it was somehow misleading.

I meant the latest patch in this ticket: attachment:0001-Include-lavu-mathematics.h-everywhere-M_PI-is-used.patch​​

Alternatively it's also on the developer list here:
​https://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2024-December/074244.html

Replying to beastd:

Could you also try to build with my latest patch applied and without the grep_or_sed gnu11 configure 's/c11/gnu11/g' (in build/media-suite_compile.sh around line 2514) of the M-AB-S fix?

The idea is that my patch makes the gnu11 part of the M-AB-S fix unnecessary.

[LigH]

I tested that patch already in comment:12, see my attachment there. I could not find a valid method to add the stdc++ lib (as you suggested in comment:13) in M-AB-S. All attempts are documented in M-AB-S issue 2702, linked in comment:14.

[beastd]

I meant did you re-test with my patch and this line of the M-AB-S fix removed?

​https://github.com/m-ab-s/media-autobuild_suite/commit/e986b33ca115517dc4e48655bb0a9db933619605#diff-adfad29200df8b40f03fd8670a191195ff4f98c0160a7b12ada8caa8cb824c75R2514

Or am I misunderstanding building MPlayer with M-AB-S works again for you since that commit?
​https://github.com/m-ab-s/media-autobuild_suite/commit/e986b33ca115517dc4e48655bb0a9db933619605

[LigH]

Building mplayer did not work, whatever I tried, until Chris Degawa commited the changes in comment:16; so I see no reason to revert it and try your patch once again, because I would be back to the fruitless efforts before that commit, it would mean to go back to comment:12 as far as I understand it.

Building mplayer works in M-AB-S as it is right now. With Chris Degawa's commit from comment:16.

[beastd]

Replying to LigH:

Building mplayer did not work, whatever I tried, until Chris Degawa commited the changes in comment:16; so I see no reason to revert it and try your patch once again, because I would be back to the fruitless efforts before that commit, it would mean to go back to comment:12 as far as I understand it.

You misunderstand me.

I want you to only revert one line of Degawa's change because that should be fixed when my patch is applied. If it works that would be confirmed.

I also want to fix the remaining problem, but I'm not clear on how to do it as of now.

Normally we do not use a C++ compiler to do the linking step and it might have undesired side effects. That's why I'm trying to understand that part of the problem and if there is possibly a better alternative.

[LigH]

Christopher Degawa is the competent person here, not me. I believe to have understood that if you want to link the C++ Standard library, you have to use the C++ linker.

I commented out the one line with grep_or_sed gnu11 configure 's/c11/gnu11/g' and the constant M_PI was unknown again.

So I un-commented that line again.

Instead, now I commented out sed -i '/%\$(EXESUF):/{n; s/\$(CC)/\$(CXX)/g};/mencoder$(EXESUF)/{n; s/\$(CC)/\$(CXX)/g}' Makefile which caused the linker to fail due to undefined references to several std:: methods (especially std::__cxx11::).

[beastd]

Replying to LigH:

I commented out the one line with grep_or_sed gnu11 configure 's/c11/gnu11/g' and the constant M_PI was unknown again.

Thanks for testing.
But was my patch applied to the MPlayer source?
If yes could you give me the compiler errors?

So I un-commented that line again.

Instead, now I commented out sed -i '/%\$(EXESUF):/{n; s/\$(CC)/\$(CXX)/g};/mencoder$(EXESUF)/{n; s/\$(CC)/\$(CXX)/g}' Makefile which caused the linker to fail due to undefined references to several std:: methods (especially std::__cxx11::).

Yes, that was to be expected. That's why I didn't ask you to do it.

[LigH]

OK, I am sorry, it was late night when I tried it. New attempt, edits in media-suite_compile.sh lines 2583-2587:

    grep_or_sed windows libmpcodecs/ad_spdif.c '/#include "mp_msg.h/ a\#include <windows.h>'
    # grep_or_sed gnu11 configure 's/c11/gnu11/g'
    do_patch "https://trac.mplayerhq.hu/raw-attachment/ticket/2423/0001-Include-lavu-mathematics.h-everywhere-M_PI-is-used.patch"
    # shellcheck disable=SC2016
    sed -i '/%\$(EXESUF):/{n; s/\$(CC)/\$(CXX)/g};/mencoder$(EXESUF)/{n; s/\$(CC)/\$(CXX)/g}' Makefile

And mplayer builds.

[beastd]

Replying to LigH:

And mplayer builds.

Ok, great. Thanks for confirming!

If there are no comments on the developer list, I plan to commit the patch to the SVN repo soon.

For the C++ linking problem I need to investigate a bit more what a solution could look like.

We cannot just use the solution from M-AB-S because MPlayer is written in C and therefore unconditionally using a C++ compiler for linking is not a good idea.

[beastd]

The name giving part (M_PI problem) of this issue is solved with MPlayer SVN r38667 .

The C++ (static) linking problem that was found as well, is not solved yet.

Thus I keep this ticket open for now.