libpjsip: remove disable-ext-sound option

PATCH New Adam Duskett 3 participants 10 messages Nov 14, 2017 – Nov 19, 2017

Share & tools

Adam Duskett

author

2017-11-14 16:07 UTC #0 lore raw

PATCH v2 1/7 libpjsip: remove disable-ext-sound option

The configure option --disable-ext-sound currently isn't a valid option in libpjsip. Originally it was used to prevent portaudio from building in the third_party directory, however, portaudio no longer exists in that directory.

Signed-off-by Adam Duskett <[email protected]>

---
Changes v1 -> v2:
- Added this patch to the start of the series.

 package/libpjsip/libpjsip.mk | 1 -
 1 file changed, 1 deletion(-)

package/libpjsip/libpjsip.mk +0 -1

index c772d41..2eaefaf 100644
@@ -37,7 +37,6 @@ LIBPJSIP_CONF_OPTS = \
 	--disable-libwebrtc \
 	--disable-opus \
 	--disable-oss \
-	--disable-ext-sound \
 	--disable-g711-codec \
 	--disable-l16-codec \
 	--disable-g722-codec \

Signature · 1 line

2.13.6

Adam Duskett

author

2017-11-14 16:07 UTC #1 lore raw

PATCH v2 2/7 libpjsip: enable alsa support

If alsa-lib is selected, libpjsip should depend on it.
Also, remove --disable-sound in the configure options or else sound would still be disabled.

Signed-off-by Adam Duskett <[email protected]>

---
Changes v1 -> v2:
- Remove the option to enable or disable alsa support in favor of just depending on alsa-lib if it's selected by the user.

 package/libpjsip/libpjsip.mk | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

package/libpjsip/libpjsip.mk +4 -1

index 2eaefaf..14dbb8e 100644
@@ -25,7 +25,6 @@ LIBPJSIP_CONF_ENV = \
 	CFLAGS="$(LIBPJSIP_CFLAGS)"
 
 LIBPJSIP_CONF_OPTS = \
-	--disable-sound \
 	--disable-gsm-codec \
 	--disable-speex-codec \
 	--disable-speex-aec \

@@ -67,4 +66,8 @@ ifeq ($(BR2_PACKAGE_UTIL_LINUX_LIBUUID),y)
 LIBPJSIP_DEPENDENCIES += util-linux
 endif
 
+ifeq ($(BR2_PACKAGE_ALSA_LIB),y)
+LIBPJSIP_DEPENDENCIES += alsa-lib
+endif
+
 $(eval $(autotools-package))

Signature · 1 line

2.13.6

Thomas Petazzoni

2017-11-14 17:00 UTC #2 lore raw

PATCH v2 2/7 libpjsip: enable alsa support

Hello,

Quoted 9 lines · Adam Duskett

If alsa-lib is selected, libpjsip should depend on it.
Also, remove --disable-sound in the configure options or else sound would still be disabled.
Signed-off-by Adam Duskett <[email protected]>
---
Changes v1 -> v2:
- Remove the option to enable or disable alsa support in favor of just depending on alsa-lib if it's selected by the user.

Why? This seems completely backward with the typical Buildroot practices.

Best regards,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Adam Duskett

author

2017-11-14 16:07 UTC #3 lore raw

PATCH v2 3/7 libpjsip: enable portaudio support

In reply to Adam Duskett

If portaudio is selected, libpjsip should depend on it. Specifying --with-external-pa was originally used to prevent libpjsip from building the portaudio library in the third_party directory, however, portaudio no longer exists in the third_party directory. This option now is used to force libpjsip to check for external portaudio headers.

Signed-off-by Adam Duskett <[email protected]>

---
Changes v1 -> v2:
- Remove the option to enable or disable portaudio support in favor of just depending on portaudio if it's selected by the user.

 package/libpjsip/libpjsip.mk | 5 +++++
 1 file changed, 5 insertions(+)

package/libpjsip/libpjsip.mk +5 -0

index 14dbb8e..3c9d789 100644
@@ -70,4 +70,9 @@ ifeq ($(BR2_PACKAGE_ALSA_LIB),y)
 LIBPJSIP_DEPENDENCIES += alsa-lib
 endif
 
+ifeq ($(BR2_PACKAGE_PORTAUDIO),y)
+LIBPJSIP_DEPENDENCIES += portaudio
+LIBPJSIP_CONF_OPTS += --with-external-pa
+endif
+
 $(eval $(autotools-package))

Signature · 1 line

2.13.6

Adam Duskett

author

2017-11-14 16:07 UTC #4 lore raw

PATCH v2 4/7 libpjsip: enable ffmpeg support

In reply to Adam Duskett

If FFmpeg is selected, libpjsip should depend on it. Specifying --with-ffmpeg= is necessary because passing --enable-FFmpeg actually disables FFmpeg when cross compiling.

Signed-off-by Adam Duskett <[email protected]>

---
Changes v1 -> v2:
- Remove the option to enable or disable FFmpeg support in favor of just depending on FFmpeg if it's selected by the user.
- Add explination as to why with-ffmpeg is being passed to libpjsip (Arnout)

 package/libpjsip/libpjsip.mk | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

package/libpjsip/libpjsip.mk +9 -2

index 3c9d789..982e088 100644
@@ -29,7 +29,6 @@ LIBPJSIP_CONF_OPTS = \
 	--disable-speex-codec \
 	--disable-speex-aec \
 	--disable-resample \
-	--disable-video \
 	--disable-opencore-amr \
 	--disable-g7221-codec \
 	--disable-ilbc-codec \

@@ -41,7 +40,6 @@ LIBPJSIP_CONF_OPTS = \
 	--disable-g722-codec \
 	--disable-libsamplerate \
 	--disable-sdl \
-	--disable-ffmpeg \
 	--disable-v4l2 \
 	--disable-openh264 \
 	--disable-libyuv \

@@ -75,4 +73,13 @@ LIBPJSIP_DEPENDENCIES += portaudio
 LIBPJSIP_CONF_OPTS += --with-external-pa
 endif
 
+# Passing --enable-ffmpeg actually disables it. Instead, --with-ffmpeg
+# must be passed to compile ffmpeg support.
+ifeq ($(BR2_PACKAGE_FFMPEG),y)
+LIBPJSIP_DEPENDENCIES += ffmpeg
+LIBPJSIP_CONF_OPTS += --with-ffmpeg=$(STAGING_DIR)/usr
+else
+LIBPJSIP_CONF_OPTS += --disable-ffmpeg
+endif
+
 $(eval $(autotools-package))

Signature · 1 line

2.13.6

Arnout Vandecappelle

2017-11-19 13:58 UTC #5 lore raw

PATCH v2 4/7 libpjsip: enable ffmpeg support

Quoted 19 lines · Adam Duskett

If FFmpeg is selected, libpjsip should depend on it. Specifying --with-ffmpeg= is necessary because passing --enable-FFmpeg actually disables FFmpeg when cross compiling.
Signed-off-by Adam Duskett <[email protected]>
---
Changes v1 -> v2:
- Remove the option to enable or disable FFmpeg support in favor of just depending on FFmpeg if it's selected by the user.
- Add explination as to why with-ffmpeg is being passed to libpjsip (Arnout)
 package/libpjsip/libpjsip.mk | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
package/libpjsip/libpjsip.mk +0 -0
index 3c9d789..982e088 100644

package/libpjsip/libpjsip.mk LIBPJSIP_CONF_OPTS = \ · line 32

Quoted 4 lines

package/libpjsip/libpjsip.mk +0 -1

@@ -29,7 +29,6 @@ LIBPJSIP_CONF_OPTS = \
 	--disable-speex-codec \
 	--disable-speex-aec \
 	--disable-resample \
-	--disable-video \

This bit should have a bit more explanation in the commit message. Like:

--enable-video has no effect at all on Linux (only on Android and darwin). --disable-video has the effect of disabling SDL, ffmpeg, v4l2, openh264 and libyuv. Since all of these are enabled/disabled explicitly already, there is no need to specify --disable-video. To do so correctly is non-trivial when we want to support e.g. ffmpeg OR v4l2.

Quoted 12 lines

package/libpjsip/libpjsip.mk +0 -1

 	--disable-opencore-amr \
 	--disable-g7221-codec \
 	--disable-ilbc-codec \

@@ -41,7 +40,6 @@ LIBPJSIP_CONF_OPTS = \
 	--disable-g722-codec \
 	--disable-libsamplerate \
 	--disable-sdl \
-	--disable-ffmpeg \
 	--disable-v4l2 \
 	--disable-openh264 \
 	--disable-libyuv \

package/libpjsip/libpjsip.mk LIBPJSIP_DEPENDENCIES += portaudio · line 77

Quoted 5 lines

package/libpjsip/libpjsip.mk +2 -0

@@ -75,4 +73,13 @@ LIBPJSIP_DEPENDENCIES += portaudio
 LIBPJSIP_CONF_OPTS += --with-external-pa
 endif
 
+# Passing --enable-ffmpeg actually disables it. Instead, --with-ffmpeg
+# must be passed to compile ffmpeg support.

Here, I would also give the full explanation in the commit message:

When --enable-ffmpeg is given, the code to enable ffmpeg is not executed. Also, when cross-compiling and no --with-ffmpeg option is given, ffmpeg is disbled. Therefore, a --with-ffmpeg option needs to be given. Normally it is used to pass the path to the directory where ffmpeg is installed, but since we install it directly under sysroot we can pass it empty.

package/libpjsip/libpjsip.mk LIBPJSIP_DEPENDENCIES += portaudio · line 80

Quoted 3 lines

package/libpjsip/libpjsip.mk +3 -0

+ifeq ($(BR2_PACKAGE_FFMPEG),y)
+LIBPJSIP_DEPENDENCIES += ffmpeg
+LIBPJSIP_CONF_OPTS += --with-ffmpeg=$(STAGING_DIR)/usr

--with-ffmpeg without anything works as well, i.e.:

LIBPJSIP_CONF_OPTS += --with-ffmpeg

Regards,
Arnout

Quoted 6 lines

package/libpjsip/libpjsip.mk +4 -0

+else
+LIBPJSIP_CONF_OPTS += --disable-ffmpeg
+endif
+
 $(eval $(autotools-package))

Signature · 6 lines

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

Adam Duskett

author

2017-11-14 16:07 UTC #6 lore raw

PATCH v2 5/7 libpjsip: enable v4l2 support

In reply to Adam Duskett

If v4l2 is selected, libpjsip should depend on it.

Signed-off-by Adam Duskett <[email protected]>

---
Changes v1 -> v2:
- Remove the option to enable or disable v4l2 support in favor of just depending on libv4l if it's selected by the user.

 package/libpjsip/libpjsip.mk | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

package/libpjsip/libpjsip.mk +6 -1

index 982e088..8b6fd09 100644
@@ -40,7 +40,6 @@ LIBPJSIP_CONF_OPTS = \
 	--disable-g722-codec \
 	--disable-libsamplerate \
 	--disable-sdl \
-	--disable-v4l2 \
 	--disable-openh264 \
 	--disable-libyuv \
 	--disable-ipp \

@@ -82,4 +81,10 @@ else
 LIBPJSIP_CONF_OPTS += --disable-ffmpeg
 endif
 
+ifeq ($(BR2_PACKAGE_LIBV4L),y)
+LIBPJSIP_DEPENDENCIES += libv4l
+else
+LIBPJSIP_CONF_OPTS += --disable-v4l2
+endif
+
 $(eval $(autotools-package))

Signature · 1 line

2.13.6

Adam Duskett

author

2017-11-14 16:07 UTC #7 lore raw

PATCH v2 6/7 libpjsip: add dependency-less codecs

In reply to Adam Duskett

libpjsip has several codecs, all of which can be disabled or enabled individually. A few reasons for adding each codec as an individual choice are:

1) Many products might not want some of the more obscure codecs such as ILBC or L16.

2) libpjsip has functions that will enumerate all the codecs that were built. If all the codecs where built, this would require a developer to have to parse out the codecs they don't want.

As such, turn libpjsip into a menuconfig and add each supported codec as an individual choice.

Start with the codecs that are built into libpjsip and have no external dependencies. These codecs are:
- G.711
- G.722
- G.722.1
- iLBC
- L16

Signed-off-by Adam Duskett <[email protected]>

---
Changes v1 -> v2:
- Combine all dependency-less codecs into a single patch.
- Add a more-detailed commit message explaining the reasoning for enabling each codec as a individual option. (Arnout)

 package/libpjsip/Config.in   | 23 ++++++++++++++++++++++-
 package/libpjsip/libpjsip.mk | 32 +++++++++++++++++++++++++++-----
 2 files changed, 49 insertions(+), 6 deletions(-)

package/libpjsip/Config.in +22 -1

index 727d2ec..d21a5b5 100644
@@ -1,4 +1,4 @@
-config BR2_PACKAGE_LIBPJSIP
+menuconfig BR2_PACKAGE_LIBPJSIP
 	bool "libpjsip"
 	depends on BR2_INSTALL_LIBSTDCPP
 	depends on BR2_TOOLCHAIN_HAS_THREADS

@@ -10,5 +10,26 @@ config BR2_PACKAGE_LIBPJSIP
 
 	  http://www.pjsip.org
 
+if BR2_PACKAGE_LIBPJSIP
+
+comment "dependency-less codecs"
+
+config BR2_PACKAGE_LIBPJSIP_CODEC_G711
+	bool "G.711"
+
+config BR2_PACKAGE_LIBPJSIP_CODEC_G722
+	bool "G.722"
+
+config BR2_PACKAGE_LIBPJSIP_CODEC_G7221
+	bool "G.722.1"
+
+config BR2_PACKAGE_LIBPJSIP_CODEC_ILBC
+	bool "iLBC"
+
+config BR2_PACKAGE_LIBPJSIP_CODEC_L16
+	bool "L16"
+
+endif #BR2_PACKAGE_LIBPJSIP
+
 comment "libpjsip needs a toolchain w/ C++, threads"
 	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS

package/libpjsip/libpjsip.mk +27 -5

index 8b6fd09..69d412a 100644
@@ -30,14 +30,9 @@ LIBPJSIP_CONF_OPTS = \
 	--disable-speex-aec \
 	--disable-resample \
 	--disable-opencore-amr \
-	--disable-g7221-codec \
-	--disable-ilbc-codec \
 	--disable-libwebrtc \
 	--disable-opus \
 	--disable-oss \
-	--disable-g711-codec \
-	--disable-l16-codec \
-	--disable-g722-codec \
 	--disable-libsamplerate \
 	--disable-sdl \
 	--disable-openh264 \

@@ -87,4 +82,31 @@ else
 LIBPJSIP_CONF_OPTS += --disable-v4l2
 endif
 
+# Codecs can only be disabled. If explicitly enabled, the check is
+# omitted (but successful), and there is no configure trace "Checking
+# if [codec] codec is disabled...no". So we only explicitly disable it
+# and we do not explicitly enable it, so we get the configure log in
+# both cases.
+ifneq ($(BR2_PACKAGE_LIBPJSIP_CODEC_G711),y)
+LIBPJSIP_CONF_OPTS += --disable-g711-codec
+endif
+
+ifneq ($(BR2_PACKAGE_LIBPJSIP_CODEC_G722),y)
+LIBPJSIP_CONF_OPTS += --disable-g722-codec
+endif
+
+# libpjsip can only use a bundled version of libg7221
+ifneq ($(BR2_PACKAGE_LIBPJSIP_CODEC_G7221),y)
+LIBPJSIP_CONF_OPTS += --disable-g7221-codec
+endif
+
+# libpjsip can only use a bundled version of libilbc
+ifneq ($(BR2_PACKAGE_LIBPJSIP_CODEC_ILBC),y)
+LIBPJSIP_CONF_OPTS += --disable-ilbc-codec
+endif
+
+ifneq ($(BR2_PACKAGE_LIBPJSIP_CODEC_L16),y)
+LIBPJSIP_CONF_OPTS += --disable-l16-codec
+endif
+
 $(eval $(autotools-package))

Signature · 1 line

2.13.6

Adam Duskett

author

2017-11-14 16:07 UTC #8 lore raw

PATCH v2 7/7 libpjsip: add codecs with dependencies

In reply to Adam Duskett

Add all supported codecs that rely on external dependencies. These codecs are:
- bcg729
- gsm
- opus
- speex

Signed-off-by Adam Duskett <[email protected]>

---
Changes v1 -> v2:
- Combine all codecs with external dependencies into a single patch.

 package/libpjsip/Config.in   | 22 ++++++++++++++++++++++
 package/libpjsip/libpjsip.mk | 32 ++++++++++++++++++++++++++++----
 2 files changed, 50 insertions(+), 4 deletions(-)

package/libpjsip/Config.in +22 -0

index d21a5b5..ef05dea 100644
@@ -29,6 +29,28 @@ config BR2_PACKAGE_LIBPJSIP_CODEC_ILBC
 config BR2_PACKAGE_LIBPJSIP_CODEC_L16
 	bool "L16"
 
+comment "Codecs with external dependencies"
+
+config BR2_PACKAGE_LIBPJSIP_CODEC_BCG729
+	bool "BCG729"
+	select BR2_PACKAGE_BCG729
+
+config BR2_PACKAGE_LIBPJSIP_CODEC_GSM
+	bool "GSM"
+	depends on !BR2_STATIC_LIBS # libgsm
+	select BR2_PACKAGE_LIBGSM
+
+comment "GSM codec needs a toolchain w/ shared libraries"
+	depends on BR2_STATIC_LIBS
+
+config BR2_PACKAGE_LIBPJSIP_CODEC_OPUS
+	bool "Opus"
+	select BR2_PACKAGE_OPUS
+
+config BR2_PACKAGE_LIBPJSIP_CODEC_SPEEX
+	bool "Speex"
+	select BR2_PACKAGE_SPEEX
+
 endif #BR2_PACKAGE_LIBPJSIP
 
 comment "libpjsip needs a toolchain w/ C++, threads"

package/libpjsip/libpjsip.mk +28 -4

index 69d412a..cfdb3af 100644
@@ -25,13 +25,9 @@ LIBPJSIP_CONF_ENV = \
 	CFLAGS="$(LIBPJSIP_CFLAGS)"
 
 LIBPJSIP_CONF_OPTS = \
-	--disable-gsm-codec \
-	--disable-speex-codec \
-	--disable-speex-aec \
 	--disable-resample \
 	--disable-opencore-amr \
 	--disable-libwebrtc \
-	--disable-opus \
 	--disable-oss \
 	--disable-libsamplerate \
 	--disable-sdl \

@@ -109,4 +105,32 @@ ifneq ($(BR2_PACKAGE_LIBPJSIP_CODEC_L16),y)
 LIBPJSIP_CONF_OPTS += --disable-l16-codec
 endif
 
+# Codecs with external dependencies
+ifeq ($(BR2_PACKAGE_LIBPJSIP_CODEC_BCG729),y)
+LIBPJSIP_DEPENDENCIES += bcg729
+LIBPJSIP_CONF_OPTS += --with-bcg729=$(STAGING_DIR)/usr
+else
+LIBPJSIP_CONF_OPTS += --disable-bcg729
+endif
+
+ifeq ($(BR2_PACKAGE_LIBPJSIP_CODEC_GSM),y)
+LIBPJSIP_DEPENDENCIES += libgsm
+LIBPJSIP_CONF_OPTS += --with-external-gsm
+else
+LIBPJSIP_CONF_OPTS += --disable-gsm-codec
+endif
+
+ifeq ($(BR2_PACKAGE_LIBPJSIP_CODEC_OPUS),y)
+LIBPJSIP_DEPENDENCIES += opus
+else
+LIBPJSIP_CONF_OPTS += --disable-opus
+endif
+
+ifeq ($(BR2_PACKAGE_LIBPJSIP_CODEC_SPEEX),y)
+LIBPJSIP_DEPENDENCIES += speex
+LIBPJSIP_CONF_OPTS += --with-external-speex
+else
+LIBPJSIP_CONF_OPTS += --disable-speex-codec --disable-speex-aec
+endif
+
 $(eval $(autotools-package))

Signature · 1 line

2.13.6

Arnout Vandecappelle

2017-11-18 17:08 UTC #9 lore raw

PATCH v2 1/7 libpjsip: remove disable-ext-sound option

In reply to Adam Duskett

Hi Adam,

Quoted 18 lines · Adam Duskett

The configure option --disable-ext-sound currently isn't a valid option in libpjsip. Originally it was used to prevent portaudio from building in the third_party directory, however, portaudio no longer exists in that directory.
Signed-off-by Adam Duskett <[email protected]>
---
Changes v1 -> v2:
- Added this patch to the start of the series.
 package/libpjsip/libpjsip.mk | 1 -
 1 file changed, 1 deletion(-)
package/libpjsip/libpjsip.mk +0 -0
index c772d41..2eaefaf 100644

package/libpjsip/libpjsip.mk LIBPJSIP_CONF_OPTS = \ · line 40

Quoted 4 lines

package/libpjsip/libpjsip.mk +0 -1

@@ -37,7 +37,6 @@ LIBPJSIP_CONF_OPTS = \
 	--disable-libwebrtc \
 	--disable-opus \
 	--disable-oss \
-	--disable-ext-sound \

I tried all combinations of the different sound options, and these are my
conclusions:

1. --enable-sound and --disable-sound both work as expected and have the effect of enabling alsa support. It has no other effect. So, I think patch 2/7 should pass --enable-sound if alsa is enabled, and --disable-sound if not.

2. --disable-ext-sound has no effect at all. However, since the option still exists, and to be future-safe in case that it may make a difference, I would not remove the option (just like you don't remove --disable-oss).

3. --with-external-pa also has no effect on the build result, other than giving the warning
<command-line>:0:0: warning: "PJMEDIA_AUDIO_DEV_HAS_PORTAUDIO" redefined

Indeed, os-auto.mk will set this define twice: it is set to 1 due to
--with-external-pa, and then set to 0 due to --enable-sound (AC_PJMEDIA_SND=alsa) or --disable-sound (AC_PJMEDIA_SND=null). The 0 "wins" so portaudio is not used at all.

Therefore, I'd remove patch 3/7 entirely and instead add an explicit
--without-external-pa.

Regards,
Arnout

Quoted 4 lines

package/libpjsip/libpjsip.mk +0 -0

 	--disable-g711-codec \
 	--disable-l16-codec \
 	--disable-g722-codec \

Signature · 6 lines

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF