[vlc-devel] [PATCH] contrib: vpx: remove strip step and fix debug symbols

Alexandre Janniaux ajanni at videolabs.io
Sat May 2 21:48:37 CEST 2020


Hi,

Just to guide reviewer, my main confusion here is on whether
or on I should add -DNDEBUG here as using --enable-debug
won't disable assertions. As assertions can be expensive it
seems reasonable that WITH_OPTIMIZATION should remove them
but it also feels wrong to only have assert in non-optimized
code. As it is not directly VLC, this is probably not that
important but I wonder what maintainers and release managers
would prefer here.

Regards,
--
Alexandre Janniaux
Videolabs

On Thu, Apr 30, 2020 at 11:39:14AM +0200, Alexandre Janniaux wrote:
> Without CONFIG_DEBUG set, libvpx is calling `$(STRIP) --strip-debug`
> on the final library and removes the debug symbols from it. With the
> variable set, it becomes a simple copy operation.
>
> In addition, debug symbols should be enabled for all builds since we
> strip in the end, so as to have the debug symbols in a separate file
> when doing releases. In case we build with optimization, remove the
> assertions.
> ---
>  contrib/src/vpx/rules.mak | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/src/vpx/rules.mak b/contrib/src/vpx/rules.mak
> index 439ee6ad372..04d4e6f0a5c 100644
> --- a/contrib/src/vpx/rules.mak
> +++ b/contrib/src/vpx/rules.mak
> @@ -144,10 +144,15 @@ ifneq ($(filter i386 x86_64,$(ARCH)),)
>  VPX_CONF += --disable-mmx
>  endif
>
> -ifndef WITH_OPTIMIZATION
> -VPX_CONF += --enable-debug --disable-optimizations
> +ifdef WITH_OPTIMIZATION
> +VPX_CFLAGS += -DNDEBUG
> +else
> +VPX_CONF += -disable-optimizations
>  endif
>
> +# Always enable debug symbols, we strip in the final executables if needed
> +VPX_CONF += --enable-debug
> +
>  ifdef HAVE_ANDROID
>  # Starting NDK19, standalone toolchains are deprecated and gcc is not shipped.
>  # The presence of gcc can be used to detect if we are using an old standalone
> @@ -162,7 +167,7 @@ endif
>  	rm -rf $(PREFIX)/include/vpx
>  	cd $< && LDFLAGS="$(VPX_LDFLAGS)" CROSS=$(VPX_CROSS) $(VPX_HOSTVARS) ./configure --target=$(VPX_TARGET) \
>  		$(VPX_CONF) --prefix=$(PREFIX)
> -	cd $< && $(MAKE)
> +	cd $< && CONFIG_DEBUG=1 $(MAKE)
>  	$(call pkg_static,"vpx.pc")
> -	cd $< && $(MAKE) install
> +	cd $< && CONFIG_DEBUG=1 $(MAKE) install
>  	touch $@
> --
> 2.26.2
>


More information about the vlc-devel mailing list