Skip to contentSkip to conversation

squeezelite: new package

PATCH Superseded kei-k at ca2.so-net.ne.jp 2 participants 3 messages Sep 28, 2015 – Sep 29, 2015

v13 of 14 Superseded Latest is v14 →

lore
Share & tools
lore

kei-k at ca2.so-net.ne.jp

author
#0 lore raw

PATCH v13 1/1 squeezelite: new package

Dear, all.

Please review attached patch.

I've fixed SQUEEZELITE_BUILD_CMDS issue.
I hope this will be the final version, and be pulled up.

Signed-off-by Hiroshi Kawashima <[email protected]>

---
v12 -> v13
- change SQUEEZELITE_SITE to github (mine), cloned from original google code site (original site is unstable to fetch).
- change to use $(TARGET_CONFIGURE_OPTS) on SQUEEZELITE_BUILD_CMDS to be suitable for buildroot make system.
- to accomplish above, change to generate buildroot suitable Makefile from original squeezelite's Makefile (using 'override' directive for CFLAGS, LDFLAGS).

v11 -> v12
- remove -DRESAMPLE_MP, because libsoxr is compiled without openMP support in buildroot, so meaningless for now.
- add -DLINKALL to resolve symbols in optional libraries (libsoxr, etc).
- add more configuration options : ffmpeg, DSD, Visualizer. lirc support is not included for now, because more work will be necessary to use it (eg: modifying package/lirc-tools to add LIRC_TOOLS_INSTALL_STAGING = YES).

---
 package/Config.in                  |    1 +
 package/squeezelite/Config.in      |   46 ++++++++++++++++++++++++++++++++
 package/squeezelite/squeezelite.mk |   51 ++++++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+), 0 deletions(-)
 create mode 100644 package/squeezelite/Config.in
 create mode 100644 package/squeezelite/squeezelite.mk
package/Config.in +1 -0
index 0e0c5cd..7c5a43a 100644
@@ -33,6 +33,7 @@ menu "Audio and video applications"
 	source "package/opus-tools/Config.in"
 	source "package/pulseaudio/Config.in"
 	source "package/sox/Config.in"
+	source "package/squeezelite/Config.in"
 	source "package/tidsp-binaries/Config.in"
 	source "package/tovid/Config.in"
 	source "package/tstools/Config.in"
package/squeezelite/Config.in new file +46 -0

46 lines not on this page · raw message

package/squeezelite/squeezelite.mk new file +51 -0

51 lines not on this page · raw message

Signature · 4 lines
1.7.1

============================================================
    Hiroshi Kawashima
#1 lore raw

PATCH v13 1/1 squeezelite: new package

Hello,

Quoted 6 lines · On Mon, 28 Sep 2015 21:46:54 +0900, kei-k at ca2.so-net.ne.jp wrote:

Dear, all.

Please review attached patch.

I've fixed SQUEEZELITE_BUILD_CMDS issue.
I hope this will be the final version, and be pulled up.

Unfortunately you are still not following the good comments from Vicente. Your commit log should *not* contain any "personal" messages. All "personal" messages should go below the "---" sign.

Quoted 15 lines
package/squeezelite/Config.in new file +9 -0
new file mode 100644
index 0000000..b4af3c8
@@ -0,0 +1,46 @@
+config BR2_PACKAGE_SQUEEZELITE
+	bool "squeezelite"
+	depends on BR2_USE_WCHAR # flac
+	depends on BR2_TOOLCHAIN_HAS_THREADS # alsa-lib
+	depends on BR2_USE_MMU # mpg123
+	select BR2_PACKAGE_ALSA_LIB
+	select BR2_PACKAGE_FLAC
+	select BR2_PACKAGE_LIBMAD
+	select BR2_PACKAGE_LIBVORBIS

package/squeezelite/Config.in line 19

Quoted 10 lines
package/squeezelite/Config.in new file +10 -0
+	select BR2_PACKAGE_FAAD2
+	select BR2_PACKAGE_MPG123
+	help
+	  Logitech Media Server client
+
+	  https://code.google.com/p/squeezelite/
+
+if BR2_PACKAGE_SQUEEZELITE
+
+config BR2_PACKAGE_SQUEEZELITE_FF

Option should be suffixed _FFMPEG

Quoted 1 line

bool "Enable wma, alac decoding (by ffmpeg)"

"(by ffmpeg)" part not needed.

Also, please use:

Enable WMA and ALAC decoding

package/squeezelite/Config.in line 24

Quoted 4 lines
package/squeezelite/Config.in new file +4 -0
+	default y
+	select BR2_PACKAGE_FFMPEG
+	help
+	  Enable wma, alac decoding

The help is useless here, so get rid of it.

package/squeezelite/Config.in line 27

Quoted 3 lines
package/squeezelite/Config.in new file +3 -0
+
+config BR2_PACKAGE_SQUEEZELITE_DSD
+	bool "Enable dsd decoding"

Please use:

"Enable Direct Speech Decoding"

or at least put DSD in capital letters.

package/squeezelite/Config.in line 32

Quoted 5 lines
package/squeezelite/Config.in new file +5 -0
+	help
+	  Enable built-in DSD decoder
+
+config BR2_PACKAGE_SQUEEZELITE_RESAMPLE
+	bool "Enable resampling function"

Rather than "function" use "support"

package/squeezelite/Config.in line 35

Quoted 3 lines
package/squeezelite/Config.in new file +3 -0
+	select BR2_PACKAGE_LIBSOXR
+	help
+	  Enable resampling function

Again help text useless if it's just copy/pasting the prompt of the option.

package/squeezelite/Config.in line 38

Quoted 3 lines
package/squeezelite/Config.in new file +3 -0
+
+config BR2_PACKAGE_SQUEEZELITE_VISEXPORT
+	bool "Enable Visualiser support"

"Enable visualiser support"

(no need for a capital V)

package/squeezelite/Config.in

Quoted 2 lines
package/squeezelite/Config.in new file +2 -0
+	help
+	  Enable Visualiser support

Useless help, should be removed.

Quoted 41 lines
package/squeezelite/Config.in new file +6 -0
+
+endif
+
+comment "squeezelite needs a toolchain w/ wchar, threads"
+	depends on BR2_USE_MMU
+	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS
package/squeezelite/squeezelite.mk new file +29 -0
new file mode 100644
index 0000000..f64da6a
@@ -0,0 +1,51 @@
+################################################################################
+#
+# squeezelite
+#
+################################################################################
+
+SQUEEZELITE_VERSION = v1.8
+SQUEEZELITE_SITE = https://github.com/robadenshi/squeezelite
+SQUEEZELITE_SITE_METHOD = git
+SQUEEZELITE_LICENSE = GPLv3
+SQUEEZELITE_LICENSE_FILE = LICENSE.txt
+SQUEEZELITE_DEPENDENCIES = alsa-lib flac libmad libvorbis faad2 mpg123
+SQUEEZELITE_MAKE_OPTS = -DLINKALL
+SQUEEZELITE_MAKEFILE  = Makefile.buildroot
+
+ifeq ($(BR2_PACKAGE_SQUEEZELITE_FF),y)
+SQUEEZELITE_DEPENDENCIES += ffmpeg
+SQUEEZELITE_MAKE_OPTS += -DFFMPEG
+endif
+
+ifeq ($(BR2_PACKAGE_SQUEEZELITE_DSD),y)
+SQUEEZELITE_MAKE_OPTS += -DDSD
+endif
+
+ifeq ($(BR2_PACKAGE_SQUEEZELITE_RESAMPLE),y)
+SQUEEZELITE_DEPENDENCIES += libsoxr
+SQUEEZELITE_MAKE_OPTS += -DRESAMPLE
+endif
+

package/squeezelite/squeezelite.mk line 39

Quoted 10 lines
package/squeezelite/squeezelite.mk new file +10 -0
+ifeq ($(BR2_PACKAGE_SQUEEZELITE_VISEXPORT),y)
+SQUEEZELITE_MAKE_OPTS += -DVISEXPORT
+endif
+
+define SQUEEZELITE_CONFIGURE_CMDS
+	sed -e '1aoverride CFLAGS += $$(OPTS)' \
+		-e 's/LDFLAGS +=/override LDFLAGS +=/' \
+		-e 's/LDFLAGS ?=/override LDFLAGS +=/' \
+		$(@D)/Makefile >$(@D)/$(SQUEEZELITE_MAKEFILE)
+endef

Don't do this. Create a patch for the Makefile that does the necessary changes.

package/squeezelite/squeezelite.mk line 48

Quoted 9 lines
package/squeezelite/squeezelite.mk new file +9 -0
+
+define SQUEEZELITE_BUILD_CMDS
+	$(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \
+		OPTS="$(SQUEEZELITE_MAKE_OPTS)" \
+		-f $(SQUEEZELITE_MAKEFILE) -C $(@D) all
+endef
+
+define SQUEEZELITE_INSTALL_TARGET_CMDS
+	$(INSTALL) -D -m 0755 $(@D)/squeezelite $(TARGET_DIR)/usr/bin

You must use a full path as the destination, i.e:

	$(TARGET_DIR)/usr/bin/squeezelite

Can you fix the above issues and send an updated version? Please be careful to take into account all the comments. If you have some doubts about certain comments, simply ask questions as a reply to this e-mail rather than sending many iterations of the patch.

Thanks a lot!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#2 lore raw

PATCH v13 1/1 squeezelite: new package

Dear Thomas.

Thank you for your comment.

I will send v14 patch with reflecting following your comment in next e-mail.

Quoted 6 lines
Quoted 2 lines

I've fixed SQUEEZELITE_BUILD_CMDS issue.
I hope this will be the final version, and be pulled up.

Unfortunately you are still not following the good comments from Vicente. Your commit log should *not* contain any "personal" messages. All "personal" messages should go below the "---" sign.

OK.

Quoted 2 lines
Quoted 1 line

+config BR2_PACKAGE_SQUEEZELITE_FF

Option should be suffixed _FFMPEG

OK.

Quoted 4 lines
Quoted 1 line

bool "Enable wma, alac decoding (by ffmpeg)"

"(by ffmpeg)" part not needed.
Also, please use:
Enable WMA and ALAC decoding

OK.

Quoted 3 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	help
+	  Enable wma, alac decoding

The help is useless here, so get rid of it.

OK. I remove it.

Quoted 4 lines

package/squeezelite/Config.in line 27

Quoted 1 line
package/squeezelite/Config.in new file +1 -0
+	bool "Enable dsd decoding"

Please use:
"Enable Direct Speech Decoding"
or at least put DSD in capital letters.

OK.
DSD is not 'Direct Speech Decoding', but 'Direct Stream Digital'. Anyway I will use DSD here.

Quoted 5 lines

package/squeezelite/Config.in line 32

Quoted 4 lines
package/squeezelite/Config.in new file +4 -0
+	help
+	  Enable built-in DSD decoder
+config BR2_PACKAGE_SQUEEZELITE_RESAMPLE
+	bool "Enable resampling function"

Rather than "function" use "support"

OK.

Quoted 5 lines

package/squeezelite/Config.in line 35

Quoted 3 lines
package/squeezelite/Config.in new file +3 -0
+	select BR2_PACKAGE_LIBSOXR
+	help
+	  Enable resampling function

Again help text useless if it's just copy/pasting the prompt of the option.

OK. removed.

Quoted 4 lines

package/squeezelite/Config.in line 38

Quoted 2 lines
package/squeezelite/Config.in new file +2 -0
+config BR2_PACKAGE_SQUEEZELITE_VISEXPORT
+	bool "Enable Visualiser support"

"Enable visualiser support"
(no need for a capital V)

OK.

Quoted 3 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	help
+	  Enable Visualiser support

Useless help, should be removed.

OK, removed.

Quoted 8 lines

package/squeezelite/squeezelite.mk line 39

Quoted 6 lines
package/squeezelite/squeezelite.mk new file +6 -0
+define SQUEEZELITE_CONFIGURE_CMDS
+	sed -e '1aoverride CFLAGS += $$(OPTS)' \
+		-e 's/LDFLAGS +=/override LDFLAGS +=/' \
+		-e 's/LDFLAGS ?=/override LDFLAGS +=/' \
+		$(@D)/Makefile >$(@D)/$(SQUEEZELITE_MAKEFILE)
+endef

Don't do this. Create a patch for the Makefile that does the necessary changes.

OK, I will make patch file.

Quoted 8 lines

package/squeezelite/squeezelite.mk line 48

Quoted 7 lines
package/squeezelite/squeezelite.mk new file +7 -0
+define SQUEEZELITE_BUILD_CMDS
+	$(TARGET_MAKE_ENV) $(MAKE) $(TARGET_CONFIGURE_OPTS) \
+		OPTS="$(SQUEEZELITE_MAKE_OPTS)" \
+		-f $(SQUEEZELITE_MAKEFILE) -C $(@D) all
+endef
+define SQUEEZELITE_INSTALL_TARGET_CMDS
+	$(INSTALL) -D -m 0755 $(@D)/squeezelite $(TARGET_DIR)/usr/bin

You must use a full path as the destination, i.e:

OK.

Quoted 8 lines

Can you fix the above issues and send an updated version? Please be careful to take into account all the comments. If you have some doubts about certain comments, simply ask questions as a reply to this e-mail rather than sending many iterations of the patch.

Thanks a lot!

Thomas

============================================================
Hiroshi Kawashima