xrdpvr was successfully recompiled at Fedora 35 (ffmpeg 4.4.3)

[alexpevzner]

Hi!

I have successfully recompiled xrdpvr at Fedore 35 (ffmpeg 4.4.3). This patch should be actually applicable to any more or less recent version of Linux. I have added missed checks for libav dependencies and added workarounds for some breaking changes in the ffmpeg libraries API

[alexpevzner]

Please, review

[matt335672]

Hi @alexpevzner

Firstly, thanks for this.

I'd like to try to get this working with the CI system, which means getting this building on Ubuntu. I'm having a few problems however, around the use of AC_CHECK_LIB when the g++ compiler is being used as a C compiler. Essentially all the checks you've added to configure.,ac are failing when CC=g++ because of name mangling.

The commit I used to add the CI is here:-

adbbbd6

There's a CI run here:-

https://github.com/matt335672/xrdp/actions/runs/3759531688

Essentially AC_CHECK_LIB mangles the names when CC=g++, and the library check fails.

One way around this might be to bas the settings for HAVE_AVFORMAT_OPEN_INPUT and friends on the value of LIBAVFORMAT_VERSION_MAJOR. Here's a search which illustrates the point, I hope:-

https://github.com/search?q=LIBAVFORMAT_VERSION_MAJOR+av_close_input_file&type=code

Thoughts?

[alexpevzner]

Hi @matt335672,

I want to try by myself.
How do you run build with gcc replaced by g++? Just make CC=g++?

[alexpevzner]

I've fixed -Werror=sign-compare error when compiling with g++. Other errors doesn't reproduce at my side

You need to install 2 packages: libavformat-dev and libavcodec-dev, not only libavformat

[matt335672]

I build with g++ by using CC=g++ ./configure <config-args>

If you duplicate the changes to build.yml and install_xrdp_dependencies_with_apt.sh which I made, you should get the same results on a push.

[alexpevzner]

Now I see.

What do you think about conditional replacement CC with gcc if it is set to g++, around AC_CHECK_LIB checks in the configure.ac?

[alexpevzner]

What do you think about conditional replacement CC with gcc if it is set to g++, around AC_CHECK_LIB checks in the configure.ac?

I have implemented this idea, and now all tests passed. Please, have a look

[matt335672]

Thanks @alexpevzner

In principle that sounds fine. I'll have a formal look when I can get some time away from the usual family commitments at this time of year!

[alexpevzner]

Thanks, @matt335672,

looking forward for you final verdict. Have a nice evening! :)

[matt335672]

Couple of minor comments/questions

[matt335672]

Suggest you remove the "for some strange reason". The actual reason is that the functions are not wrapped in a extern "C" wrapper, so the C++ compiler will mangle them. See this link for more explanation.

[matt335672]

There's a few of these about. Presumably it's for compatibility with older versions of the library. Is that right?

[alexpevzner]

Hi @matt335672,

Suggest you remove the "for some strange reason".

autoconf generates test files with the .c extention, and g++ 11.3.1 on Fedora treats them as C language files, not as C++. So it looks strange for me that g++ on Ubuntu doesn't do the same

Presumably it's for compatibility with older versions of the library. Is that right?

Not exactly. These functions are marked as "deprecated" at the current version of the library, so they are likely to disappear in the reasonable future. Where possible, I've found a non-deprecated replacement, in another places I've just silenced the warning, but eventually these places must be somehow fixed.

[matt335672]

I'm not seeing that behaviour on a live Fedora 37 CD with g++ installed. On that platform, g++ will mangle external C names:-

$ cat temp.c
int externfunc();

int main()
{
    return externfunc();
}
$ gcc -c temp.c
$ nm temp.o | grep externfunc
                 U externfunc
$ g++ -c temp.c
$ nm temp.o | grep externfunc
                 U _Z10externfuncv
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/12/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-redhat-linux
Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,fortran,objc,obj-c++,ada,go,d,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-gcc-major-version-only --enable-libstdcxx-backtrace --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --with-isl=/builddir/build/BUILD/gcc-12.2.1-20221121/obj-x86_64-redhat-linux/isl-install --enable-offload-targets=nvptx-none --without-cuda-driver --enable-offload-defaulted --enable-gnu-indirect-function --enable-cet --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux --with-build-config=bootstrap-lto --enable-link-serialization=1
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.2.1 20221121 (Red Hat 12.2.1-4) (GCC) 

As far as the "deprecated" warnings go, are you able to comment them with the version at which the deprecation warning was introduced? That will help all of us trying to maintain this going forwards.

Thanks.

[matt335672]

Hi @alexpevzner

We seem to have stalled on this one. Given my comment above, are you happy to remove the "for some strange reason" and squash so we can get this one put to bed?

Thanks.