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.

Sign up for GitHub

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

Jump to bottom

remove native nasm #6263

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

remove native nasm #6263

wants to merge 11 commits into from

Conversation

hgy59
Copy link
Contributor

@hgy59 hgy59 commented Oct 7, 2024 •
edited
Loading

Description

  • remove native nasm, it is a prerequisite in the dev. environment since update to debian 12
  • remove native yasm
  • update related Makefiles

Fixes #

Affected Libraries

Direct dependencies

  • cross/dav1d
  • cross/ffmpeg*
  • cross/lame
  • cross/libaom
  • cross/libvpx
  • cross/openh264
  • cross/svt-av1
  • cross/x264
  • cross/x265

Further findings

  • cross/zstd

Affected Packages

By direct dependencies

  • ffmpeg*
  • chromaprint
  • comskip
  • tvheadend
  • imagemagick
  • mpd
  • rutorrent

Checklist

  • Build rule all-supported completed successfully
  • New installation of package completed successfully
  • Package upgrade completed successfully (Manually install the package again)
  • Package functionality was tested
  • Any needed documentation is updated/created

Type of change

  • Includes small framework changes

hgy59 added 5 commits October 7, 2024 21:55
@hgy59
remove native nasm
7889de7 
- remove native nasm, it is a prerequisite in the dev. environment since update to debian 12
- remove native yasm
- update related Makefiles
@hgy59
temp add output to analyze prepare action
2c4a9cb
@hgy59
Revert "temp add output to analyze prepare action"
d880967 
This reverts commit 2c4a9cb.
@hgy59
avoid variable in DEPENDS
448c879 
- change affected packages to trigger build
@hgy59
revert unintended changes
52b31b8
@hgy59 hgy59 requested a review from th0ma7 October 9, 2024 19:05
th0ma7
th0ma7 requested changes Oct 9, 2024
View reviewed changes
Copy link
Contributor

@th0ma7 th0ma7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beside possible issue found with openh264 that your change brought to light, LGTM.

cross/openh264/Makefile Outdated Show resolved Hide resolved
hgy59 added 4 commits December 17, 2024 18:24
@hgy59
Merge branch 'master' of https://github.com/hgy59/spksrc into remove_…
db7c4ba 
…native_nasm
@hgy59
cross/dav1d: update from v1.4.2 to v1.4.3
e5b50fa 
- update cross/dav1d
- remove definitions for nasm, meson build completely auto detects nasm
@hgy59
cross/openh264: remove nasm definitions
db9b00f 
- remove definitions for nasm, meson build completely auto detects nasm
- create optimized code
@hgy59
remove CMAKE_USE_NASM
f7381e6 
- cmake builds completely auto detect nasm
@hgy59
Copy link
Contributor Author

hgy59 commented Dec 17, 2024

Special NASM definitions for meson and cmake build systems are obsolete and removed now.
The auto configuration for nasm works as expected.

further analysis for gcc builds (spksrc.cross-cc.mk) is WIP.

hgy59 added 2 commits December 19, 2024 19:37
@hgy59
cross/x264: update build
bffe33b 
- disable support for OLD_PPC_ARCHS
- build with assembler, except for PPC_ARCHS
- remove obsolete configure args
  --enable-pic is forced with --enable-shared
  --cross-prefix is not required
- remove definition of GNU_CONFIGURE and define --host
- simplify definition of gcc as assembler if not using x86asm
@hgy59
update build of cross/libvpx
49d2ca7 
- define --as=auto to use yasmm (prefered over nasm)
- document why we don't define GNU_CONFIGRE
- remove concurring patch
- avoid duplicate definition of --disable-neon
hgy59
hgy59 commented Dec 20, 2024
View reviewed changes
cross/libvpx/Makefile
ifeq ($(findstring $(ARCH),$(i686_ARCHS)),$(ARCH))
CONFIGURE_ARGS += --target=x86-linux-gcc
# --as=auto: prefers yasm over nasm
CONFIGURE_ARGS += --as=auto
CONFIGURE_ARGS += --disable-sse4_1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@th0ma7 can you remember why you disabled sse4_1 for intel archs?
it builds just fine without disabling it

@hgy59
Copy link
Contributor Author

hgy59 commented Dec 20, 2024

@th0ma7 the libraries and *.mk files are ready for review.

On the ffmpeg* packages there is some pending work...

  1. OLD_PPC_ARCHS fail to build ffmpeg since 4.4.x (4.3.x are the last working releases, newer have issue with stdatomic)

  2. There are some conditional dependencies for alpine comcerto2k and monaco archs.

    example:

    ifeq ($(findstring $(ARCH),alpine comcerto2k monaco $(ARMv8_ARCHS) $(i686_ARCHS) $(x64_ARCHS)),$(ARCH))
DEPENDS += cross/libaom
CONFIGURE_ARGS += --enable-libaom
endif

    Except for arch-comcerto2k-7.1 we do not build additional packages for alpine and monaco
    If we would build dedicated packages for alpine and monaco we would need to exclude those archs in the armv7 generic packages.
    Despite we have an option to exclude dedicated archs from generic ones with REMOVE_FROM_GENERIC_ARCHS, we cannot use github build to create and publish dedicated archs.
    And we never planned to created a single package for a list of dedicated archs...

@hgy59
Copy link
Contributor Author

hgy59 commented Dec 20, 2024

and the third issue is with cross/lame:

even we configure with --enable-asm (and configure finds /usr/bin/nasm) it does not use the assembler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@th0ma7 th0ma7 th0ma7 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hgy59 @th0ma7