Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asm generic termbits.h macro "B0" collides with other macros #630

Closed
matthewn4444 opened this issue Jan 26, 2018 · 7 comments
Closed

asm generic termbits.h macro "B0" collides with other macros #630

matthewn4444 opened this issue Jan 26, 2018 · 7 comments
Assignees

Comments

@matthewn4444
Copy link

matthewn4444 commented Jan 26, 2018

Description

Compiling with libraries such as (newest) FFMPEG with arm64-v8a collides with several functions of the macro B0 (such as hevc_mvs.c). You can see how it may cause issues in this person's blog: http://alientechlab.com/how-to-build-ffmpeg-for-android/

Basically what happens is sysroot/usr/include/asm-generic/termbits.h defines a macro definition of

#define B0 0000000000

and FFMPEG uses variables such as

 is_available_B0 = AVAILABLE(cand_up_right, B0) &&
            xb0 < s->ps.sps->width &&
            PRED_BLOCK_AVAILABLE(B0) &&
            !is_diff_mer(s, xB0, yB0, x0, y0);

And then replaces them with 0000000000 as

 is_available_0000000000  = AVAILABLE(cand_up_right, 0000000000 ) &&
            x0000000000  < s->ps.sps->width &&
            PRED_BLOCK_AVAILABLE(0000000000 ) &&
            !is_diff_mer(s, x0000000000 , y0000000000 , x0, y0);

Which then leads to compile issues like

libavcodec/hevc_mvs.c: In function 'derive_spatial_merge_candidates':
libavcodec/hevc_mvs.c:368:23: error: 'y0000000' undeclared (first use in this function)
libavcodec/hevc_mvs.c:368:23: note: each undeclared identifier is reported only once for each function it appears in
libavcodec/hevc_mvs.c:368:23: error: 'x0000000' undeclared (first use in this function)
libavcodec/hevc_mvs.c: In function 'ff_hevc_luma_mv_mvp_mode':
libavcodec/hevc_mvs.c:683:24: error: 'y0000000' undeclared (first use in this function)
libavcodec/hevc_mvs.c:683:24: error: 'x0000000' undeclared (first use in this function)
ffbuild/common.mak:60: recipe for target 'libavcodec/hevc_mvs.o' failed

Sorry for my ignorance but does B0 have some special reason for being 00000000? Could this be changed in the future since this could potentially have collisions with other libraries (for only being 2 letters). (I am not sure if FFMPEG wants to rewrite their variables).

For now, I am using a script to comment that line out (from the termbits.h).

To reproduce you can follow the instructions in that blog (clone FFMPEG, configure, make and make install)

Environment Details

  • NDK Version: r16b
  • Build sytem: ndk-build, standalone tool chain
  • Host OS: Linux (Windows 10 Ubuntu shell, I think it is 16.04)
  • Compiler: Clang
  • ABI: arm64-v8a
  • STL: libc++
  • NDK API level: 27
  • Device API level: N/A, not compiling for device
@rprichard
Copy link
Collaborator

From the blog post, the issue appears to be:

  • FFMPEG has a B0 local variable.
  • It includes sys/ioctl.h, which includes asm/termbits.h.
  • asm/termbits.h defines a B0 macro.

This code doesn't compile with Bionic, but it does with glibc, musl, and FreeBSD:

#include <sys/ioctl.h>
int main() {
  int B0 = 0;
}

Maybe sys/ioctl.h shouldn't include asm/termbits.h.

@enh
Copy link
Contributor

enh commented Jan 26, 2018

Maybe sys/ioctl.h shouldn't include asm/termbits.h.

sounds plausible to me, based on what that #include gets us. there was no motivation in the git history either.

the only issue will be compatibility with ourselves. testing that for the platform now: https://android-review.googlesource.com/#/c/platform/bionic/+/600968/

@enh enh self-assigned this Jan 26, 2018
@enh
Copy link
Contributor

enh commented Jan 26, 2018

Sorry for my ignorance but does B0 have some special reason for being 00000000?

(it's one of a number of baud rate constants for serial lines. there's B0, B300, B2400, et cetera. it's a legit POSIX constant, but POSIX says it's in <termios.h>.)

@matthewn4444
Copy link
Author

matthewn4444 commented Jan 26, 2018

Cool, would the change make it in the next version of the NDK?

Edit: I applied that change to the standalone toolchain similar to the change above and I still get the error. Based on the review change above, also removing #include <linux/termios.h> from sys/ioctl.h allowed it to build.

hubot pushed a commit to aosp-mirror/platform_bionic that referenced this issue Jan 26, 2018
The history has no motivation for <asm/termbits.h>, and other C libraries
don't seem to include any of the extra types and constants that gains us.

This caused problems building FFMPEG, which defines its own B0.

Bug: android/ndk#630
Test: builds
Change-Id: If032b9fa7860777c13f7bd8e68fb78ff606dcd94
@enh
Copy link
Contributor

enh commented Jan 26, 2018

that's more of a problem because <sys/ioctl.h> should expose struct winsize and the TIOCM_ constants, and they come from #include <linux/termios.h>. i'm not sure there's anything we can do about that, other than suggest that ffmpeg shouldn't try to reuse names reserved by POSIX. (<sys/ioctl.h> isn't a POSIX header, so there are no guarantees about which other headers it causes to be included.) closing this bug as "infeasible" unless someone has a clever idea...

btw, that page you linked to isn't the right way to build configure-based projects with the NDK (you can tell because it installs a different compiler). see https://developer.android.com/ndk/guides/standalone_toolchain.html#building_open_source_projects_using_standalone_toolchains for how it should actually work.

@enh enh closed this as completed Jan 26, 2018
@matthewn4444
Copy link
Author

matthewn4444 commented Jan 27, 2018

For anyone reading this in the future, to prevent this without modifying the ndk or ffmpeg code, use

--disable-linux-perf

It will make /ffmpeg/ffbuild/config.mak comment out the line CONFIG_LINUX_PERF. This prevents the code in /libavutil/timer.h to include the header <sys/ioctl.h> and it will build (which imports <linux/termios.h> and has the B0 constant).

@jiangjie8
Copy link

@matthewn4444 Very good solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants