Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] package/qt6: add qt6 multimedia
@ 2024-08-10 17:13 baxiche
  2024-08-14 11:59 ` Roy Kollen Svendsen
  0 siblings, 1 reply; 4+ messages in thread
From: baxiche @ 2024-08-10 17:13 UTC (permalink / raw)
  To: buildroot
  Cc: baxiche su, Jesse Van Gavere, Samuel Martin, Roy Kollen Svendsen,
	Thomas Petazzoni

From: baxiche su <baxiche@gmail.com>

Signed-off-by: baxiche su <baxiche@gmail.com>
---
 package/qt6/Config.in                        |  1 +
 package/qt6/qt6multimedia/Config.in          | 15 +++++
 package/qt6/qt6multimedia/qt6multimedia.hash | 13 ++++
 package/qt6/qt6multimedia/qt6multimedia.mk   | 71 ++++++++++++++++++++
 4 files changed, 100 insertions(+)
 create mode 100644 package/qt6/qt6multimedia/Config.in
 create mode 100644 package/qt6/qt6multimedia/qt6multimedia.hash
 create mode 100644 package/qt6/qt6multimedia/qt6multimedia.mk

diff --git a/package/qt6/Config.in b/package/qt6/Config.in
index 29c1c25c7f..5b55e67b26 100644
--- a/package/qt6/Config.in
+++ b/package/qt6/Config.in
@@ -57,5 +57,6 @@ source "package/qt6/qt6tools/Config.in"
 source "package/qt6/qt6virtualkeyboard/Config.in"
 source "package/qt6/qt6wayland/Config.in"
 source "package/qt6/qt6websockets/Config.in"
+source "package/qt6/qt6multimedia/Config.in"
 
 endif
diff --git a/package/qt6/qt6multimedia/Config.in b/package/qt6/qt6multimedia/Config.in
new file mode 100644
index 0000000000..3d0bf4c70b
--- /dev/null
+++ b/package/qt6/qt6multimedia/Config.in
@@ -0,0 +1,15 @@
+config BR2_PACKAGE_QT6MULTIMEDIA
+	bool "qt6multimedia"
+	select BR2_PACKAGE_QT6BASE_GUI
+	select BR2_PACKAGE_QT6BASE_NETWORK
+	select BR2_PACKAGE_QT6BASE_WIDGETS
+	select BR2_PACKAGE_QT6BASE_CONCURRENT
+	select BR2_PACKAGE_QT6SHADERTOOLS
+
+	help
+	  Qt is a cross-platform application and UI framework for
+	  developers using C++.
+
+	  This package corresponds to the qt6multimedia module.
+
+	  https://doc.qt.io/qt-6/qtmultimedia-index.html
diff --git a/package/qt6/qt6multimedia/qt6multimedia.hash b/package/qt6/qt6multimedia/qt6multimedia.hash
new file mode 100644
index 0000000000..04a8c0cb7d
--- /dev/null
+++ b/package/qt6/qt6multimedia/qt6multimedia.hash
@@ -0,0 +1,13 @@
+# Hash from: https://download.qt.io/official_releases/qt/6.7/6.7.2/submodules/qtmultimedia-everywhere-src-6.7.2.tar.xz.sha256
+sha256  8ef835115acb9a1d3d2c9f23cfacb43f2c537e3786a8ab822299a2a7765651d3  qtmultimedia-everywhere-src-6.7.2.tar.xz
+
+# Hashes for license files:
+sha256  9f0490f18656c6f2435bd14f603ef0c96434d1825615363dce43abb42ed1dcce  LICENSES/BSD-3-Clause.txt
+sha256  3abd6471b9a9a08d65ce771143f8e278bb4c1aeb10c1c2d476935a6b049653f5  LICENSES/BSL-1.0.txt
+sha256  110535522396708cea37c72a802c5e7e81391139f5f7985631c93ef242b206a4  LICENSES/GFDL-1.3-no-invariants-only.txt
+sha256  8177f97513213526df2cf6184d8ff986c675afb514d4e68a404010521b880643  LICENSES/GPL-2.0-only.txt
+sha256  8ceb4b9ee5adedde47b31e975c1d90c73ad27b6b165a1dcd80c7c545eb65b903  LICENSES/GPL-3.0-only.txt
+sha256  da7eabb7bafdf7d3ae5e9f223aa5bdc1eece45ac569dc21b3b037520b4464768  LICENSES/LGPL-3.0-only.txt
+sha256  d040359701c01996a017d5c515678929cd1b0625e64cb86db44025fbb6cdf1fc  LICENSES/MIT.txt
+sha256  9b1f50aae6267f9d5e0ceb6775ee86450262c25ec7c0573e151fe5d3f18a4700  LICENSES/LicenseRef-Qt-Commercial.txt
+sha256  40678d338ce53cd93f8b22b281a2ecbcaa3ee65ce60b25ffb0c462b0530846b2  LICENSES/Qt-GPL-exception-1.0.txt
\ No newline at end of file
diff --git a/package/qt6/qt6multimedia/qt6multimedia.mk b/package/qt6/qt6multimedia/qt6multimedia.mk
new file mode 100644
index 0000000000..a7e9bd22a0
--- /dev/null
+++ b/package/qt6/qt6multimedia/qt6multimedia.mk
@@ -0,0 +1,71 @@
+################################################################################
+#
+# qt6multimedia
+#
+################################################################################
+
+QT6MULTIMEDIA_VERSION = $(QT6_VERSION)
+QT6MULTIMEDIA_SITE = $(QT6_SITE)
+QT6MULTIMEDIA_SOURCE = qtmultimedia-$(QT6_SOURCE_TARBALL_PREFIX)-$(QT6MULTIMEDIA_VERSION).tar.xz
+QT6MULTIMEDIA_INSTALL_STAGING = YES
+
+QT6MULTIMEDIA_SUPPORTS_IN_SOURCE_BUILD = NO
+
+QT6MULTIMEDIA_CMAKE_BACKEND = ninja
+
+QT6MULTIMEDIA_LICENSE = \
+	LGPL-3.0 or GPL-2.0 or GPL-3.0, \
+	LGPL-2.1+, \
+	BSD-3-Clause (docs), \
+	GFDL-1.3 no invariants (docs), \
+	BSD-3-Clause (examples), \
+	BSL-1.0, \
+	MPL-2.0 \
+	MIT
+
+QT6MULTIMEDIA_LICENSE_FILES = \
+	LICENSES/GPL-2.0-only.txt \
+	LICENSES/GPL-3.0-only.txt \
+	LICENSES/LGPL-3.0-only.txt \
+	LICENSES/GFDL-1.3-no-invariants-only.txt \
+	LICENSES/BSL-1.0.txt \
+	LICENSES/BSD-3-Clause.txt \
+	LICENSES/MIT.txt \
+	LICENSES/Qt-GPL-exception-1.0.txt \
+	LICENSES/MPL-2.0.txt \
+	LICENSES/IJG.txt \
+	LICENSES/ISC.txt \
+	LICENSES/zlib.txt \
+
+QT6MULTIMEDIA_CONF_OPTS = \
+	-DQT_HOST_PATH=$(HOST_DIR) \
+	-DBUILD_WITH_PCH=OFF \
+	-DQT_BUILD_EXAMPLES=OFF \
+	-DQT_BUILD_TESTS=OFF
+
+QT6MULTIMEDIA_DEPENDENCIES = \
+	qt6base \
+	qt6shadertools
+
+ifeq ($(BR2_PACKAGE_QT6DECLARATIVE),y)
+QT6MULTIMEDIA_DEPENDENCIES += qt6declarative
+endif
+
+ifeq ($(BR2_PACKAGE_PULSEAUDIO),y)
+QT6MULTIMEDIA_DEPENDENCIES += pulseaudio
+endif
+
+ifeq ($(BR2_PACKAGE_FFMPEG)$(BR2_PACKAGE_PULSEAUDIO),yy)
+QT6MULTIMEDIA_CONF_OPTS += -DFFMPEG_DIR=$(BUILD_DIR)/ffmpeg-$(FFMPEG_VERSION) -DQT_DEPLOY_FFMPEG=ON
+QT6MULTIMEDIA_DEPENDENCIES += ffmpeg
+endif
+
+ifeq ($(BR2_PACKAGE_GSTREAMER1),y)
+QT6MULTIMEDIA_DEPENDENCIES += gstreamer1
+endif
+
+ifeq ($(BR2_PACKAGE_LIBVA),y)
+QT6MULTIMEDIA_DEPENDENCIES += libva
+endif
+
+$(eval $(cmake-package))
-- 
2.45.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [Buildroot] [PATCH 1/1] package/qt6: add qt6 multimedia
  2024-08-10 17:13 [Buildroot] [PATCH 1/1] package/qt6: add qt6 multimedia baxiche
@ 2024-08-14 11:59 ` Roy Kollen Svendsen
  2024-09-14 10:13   ` Thomas Petazzoni via buildroot
  0 siblings, 1 reply; 4+ messages in thread
From: Roy Kollen Svendsen @ 2024-08-14 11:59 UTC (permalink / raw)
  To: baxiche; +Cc: Jesse Van Gavere, Samuel Martin, Thomas Petazzoni, buildroot

Hi baxiche,

Thank you, I needed this package.

And I have a few comments:

lør. 10. aug. 2024 kl. 19:13 skrev <baxiche@gmail.com>:
>
> From: baxiche su <baxiche@gmail.com>
>
> Signed-off-by: baxiche su <baxiche@gmail.com>
> ---
>  package/qt6/Config.in                        |  1 +
>  package/qt6/qt6multimedia/Config.in          | 15 +++++
>  package/qt6/qt6multimedia/qt6multimedia.hash | 13 ++++
>  package/qt6/qt6multimedia/qt6multimedia.mk   | 71 ++++++++++++++++++++
>  4 files changed, 100 insertions(+)
>  create mode 100644 package/qt6/qt6multimedia/Config.in
>  create mode 100644 package/qt6/qt6multimedia/qt6multimedia.hash
>  create mode 100644 package/qt6/qt6multimedia/qt6multimedia.mk
>
> diff --git a/package/qt6/Config.in b/package/qt6/Config.in
> index 29c1c25c7f..5b55e67b26 100644
> --- a/package/qt6/Config.in
> +++ b/package/qt6/Config.in
> @@ -57,5 +57,6 @@ source "package/qt6/qt6tools/Config.in"
>  source "package/qt6/qt6virtualkeyboard/Config.in"
>  source "package/qt6/qt6wayland/Config.in"
>  source "package/qt6/qt6websockets/Config.in"
> +source "package/qt6/qt6multimedia/Config.in"

I think these should be ordered alphabetically

>
>  endif
> diff --git a/package/qt6/qt6multimedia/Config.in b/package/qt6/qt6multimedia/Config.in
> new file mode 100644
> index 0000000000..3d0bf4c70b
> --- /dev/null
> +++ b/package/qt6/qt6multimedia/Config.in
> @@ -0,0 +1,15 @@
> +config BR2_PACKAGE_QT6MULTIMEDIA
> +       bool "qt6multimedia"
> +       select BR2_PACKAGE_QT6BASE_GUI
> +       select BR2_PACKAGE_QT6BASE_NETWORK
> +       select BR2_PACKAGE_QT6BASE_WIDGETS
> +       select BR2_PACKAGE_QT6BASE_CONCURRENT
> +       select BR2_PACKAGE_QT6SHADERTOOLS

Probably does not hurt to order these alphabetically also

> +
> +       help
> +         Qt is a cross-platform application and UI framework for
> +         developers using C++.
> +
> +         This package corresponds to the qt6multimedia module.
> +
> +         https://doc.qt.io/qt-6/qtmultimedia-index.html
> diff --git a/package/qt6/qt6multimedia/qt6multimedia.hash b/package/qt6/qt6multimedia/qt6multimedia.hash
> new file mode 100644
> index 0000000000..04a8c0cb7d
> --- /dev/null
> +++ b/package/qt6/qt6multimedia/qt6multimedia.hash
> @@ -0,0 +1,13 @@
> +# Hash from: https://download.qt.io/official_releases/qt/6.7/6.7.2/submodules/qtmultimedia-everywhere-src-6.7.2.tar.xz.sha256
> +sha256  8ef835115acb9a1d3d2c9f23cfacb43f2c537e3786a8ab822299a2a7765651d3  qtmultimedia-everywhere-src-6.7.2.tar.xz
> +
> +# Hashes for license files:
> +sha256  9f0490f18656c6f2435bd14f603ef0c96434d1825615363dce43abb42ed1dcce  LICENSES/BSD-3-Clause.txt
> +sha256  3abd6471b9a9a08d65ce771143f8e278bb4c1aeb10c1c2d476935a6b049653f5  LICENSES/BSL-1.0.txt
> +sha256  110535522396708cea37c72a802c5e7e81391139f5f7985631c93ef242b206a4  LICENSES/GFDL-1.3-no-invariants-only.txt
> +sha256  8177f97513213526df2cf6184d8ff986c675afb514d4e68a404010521b880643  LICENSES/GPL-2.0-only.txt
> +sha256  8ceb4b9ee5adedde47b31e975c1d90c73ad27b6b165a1dcd80c7c545eb65b903  LICENSES/GPL-3.0-only.txt
> +sha256  da7eabb7bafdf7d3ae5e9f223aa5bdc1eece45ac569dc21b3b037520b4464768  LICENSES/LGPL-3.0-only.txt
> +sha256  d040359701c01996a017d5c515678929cd1b0625e64cb86db44025fbb6cdf1fc  LICENSES/MIT.txt
> +sha256  9b1f50aae6267f9d5e0ceb6775ee86450262c25ec7c0573e151fe5d3f18a4700  LICENSES/LicenseRef-Qt-Commercial.txt
> +sha256  40678d338ce53cd93f8b22b281a2ecbcaa3ee65ce60b25ffb0c462b0530846b2  LICENSES/Qt-GPL-exception-1.0.txt
> \ No newline at end of file
> diff --git a/package/qt6/qt6multimedia/qt6multimedia.mk b/package/qt6/qt6multimedia/qt6multimedia.mk
> new file mode 100644
> index 0000000000..a7e9bd22a0
> --- /dev/null
> +++ b/package/qt6/qt6multimedia/qt6multimedia.mk
> @@ -0,0 +1,71 @@
> +################################################################################
> +#
> +# qt6multimedia
> +#
> +################################################################################
> +
> +QT6MULTIMEDIA_VERSION = $(QT6_VERSION)
> +QT6MULTIMEDIA_SITE = $(QT6_SITE)
> +QT6MULTIMEDIA_SOURCE = qtmultimedia-$(QT6_SOURCE_TARBALL_PREFIX)-$(QT6MULTIMEDIA_VERSION).tar.xz
> +QT6MULTIMEDIA_INSTALL_STAGING = YES
> +
> +QT6MULTIMEDIA_SUPPORTS_IN_SOURCE_BUILD = NO
> +
> +QT6MULTIMEDIA_CMAKE_BACKEND = ninja
> +
> +QT6MULTIMEDIA_LICENSE = \
> +       LGPL-3.0 or GPL-2.0 or GPL-3.0, \
> +       LGPL-2.1+, \
> +       BSD-3-Clause (docs), \
> +       GFDL-1.3 no invariants (docs), \
> +       BSD-3-Clause (examples), \
> +       BSL-1.0, \
> +       MPL-2.0 \
> +       MIT

Thomas, should we order these licenses alpabetically, and avoid
duplicating the lines?:

To avoid duplicating lines I would do:

QT6MULTIMEDIA_LICENSE = \
      LGPL-3.0 or GPL-2.0 or GPL-3.0, \
      LGPL-2.1+, \
      BSD-3-Clause (docs, examples), \
      GFDL-1.3 no invariants (docs), \
      BSL-1.0, \
      MPL-2.0 \
      MIT

And order alphabetically:
      BSD-3-Clause (docs, examples), \
      BSL-1.0, \
      GFDL-1.3 no invariants (docs), \
      LGPL-2.1+, \
      LGPL-3.0 or GPL-2.0 or GPL-3.0, \
      MIT \
      MPL-2.0

This will also make it easier to do some autogeneration and
verification to decide if license information is correct.

> +
> +QT6MULTIMEDIA_LICENSE_FILES = \
> +       LICENSES/GPL-2.0-only.txt \
> +       LICENSES/GPL-3.0-only.txt \
> +       LICENSES/LGPL-3.0-only.txt \
> +       LICENSES/GFDL-1.3-no-invariants-only.txt \
> +       LICENSES/BSL-1.0.txt \
> +       LICENSES/BSD-3-Clause.txt \
> +       LICENSES/MIT.txt \
> +       LICENSES/Qt-GPL-exception-1.0.txt \
> +       LICENSES/MPL-2.0.txt \
> +       LICENSES/IJG.txt \
> +       LICENSES/ISC.txt \
> +       LICENSES/zlib.txt \
> +

I think these lines should be ordered like this:

$ ls -1 LICENSES/ | awk '{print "\tLICENSES/"$1" \\"}'
    LICENSES/BSD-2-Clause.txt \
    LICENSES/BSD-3-Clause.txt \
    LICENSES/BSD-Source-Code.txt \
    LICENSES/BSL-1.0.txt \
    LICENSES/GFDL-1.3-no-invariant
s-only.txt \
    LICENSES/GPL-2.0-only.txt \
    LICENSES/GPL-3.0-only.txt \
    LICENSES/IJG.txt \
    LICENSES/ISC.txt \
    LICENSES/LGPL-2.1-or-later.txt \
    LICENSES/LGPL-3.0-only.txt \
    LICENSES/LicenseRef-Qt-Commercial.txt \
    LICENSES/MIT.txt \
    LICENSES/MPL-2.0.txt \
    LICENSES/Qt-GPL-exception-1.0.txt \
    LICENSES/Zlib.txt

> +QT6MULTIMEDIA_CONF_OPTS = \
> +       -DQT_HOST_PATH=$(HOST_DIR) \
> +       -DBUILD_WITH_PCH=OFF \
> +       -DQT_BUILD_EXAMPLES=OFF \
> +       -DQT_BUILD_TESTS=OFF
> +
> +QT6MULTIMEDIA_DEPENDENCIES = \
> +       qt6base \
> +       qt6shadertools
> +
> +ifeq ($(BR2_PACKAGE_QT6DECLARATIVE),y)
> +QT6MULTIMEDIA_DEPENDENCIES += qt6declarative
> +endif

I think it is custom to order these ifeq-statements alphabetically?

> +
> +ifeq ($(BR2_PACKAGE_PULSEAUDIO),y)
> +QT6MULTIMEDIA_DEPENDENCIES += pulseaudio
> +endif
> +
> +ifeq ($(BR2_PACKAGE_FFMPEG)$(BR2_PACKAGE_PULSEAUDIO),yy)

It may be better to avoid doing the BR2_PACKAGE_PULSEAUDIO==y twice,
by moving the BR2_PACKAGE_FFMPEG==y inside ifeq
($(BR2_PACKAGE_PULSEAUDIO),y)-block above.

> +QT6MULTIMEDIA_CONF_OPTS += -DFFMPEG_DIR=$(BUILD_DIR)/ffmpeg-$(FFMPEG_VERSION) -DQT_DEPLOY_FFMPEG=ON

What about splitting the long line to increase readability?:

QT6MULTIMEDIA_CONF_OPTS += \
      -DFFMPEG_DIR=$(BUILD_DIR)/ffmpeg-$(FFMPEG_VERSION) \
      -DQT_DEPLOY_FFMPEG=ON

And maybe add an explainations in the commit message and in the
mk-file why you need to set these options?

> +QT6MULTIMEDIA_DEPENDENCIES += ffmpeg
> +endif
> +
> +ifeq ($(BR2_PACKAGE_GSTREAMER1),y)
> +QT6MULTIMEDIA_DEPENDENCIES += gstreamer1
> +endif
> +
> +ifeq ($(BR2_PACKAGE_LIBVA),y)
> +QT6MULTIMEDIA_DEPENDENCIES += libva
> +endif
> +
> +$(eval $(cmake-package))
> --
> 2.45.1
>

Regards,
Roy
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [Buildroot] [PATCH 1/1] package/qt6: add qt6 multimedia
  2024-08-14 11:59 ` Roy Kollen Svendsen
@ 2024-09-14 10:13   ` Thomas Petazzoni via buildroot
  2024-09-15 17:05     ` baxiche su
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Petazzoni via buildroot @ 2024-09-14 10:13 UTC (permalink / raw)
  To: Roy Kollen Svendsen; +Cc: baxiche, Jesse Van Gavere, Samuel Martin, buildroot

Hello Baxiche, Hello Roy,

First of all, thanks Roy for the review, much appreciated! See my
comments below.

On Wed, 14 Aug 2024 13:59:04 +0200
Roy Kollen Svendsen <roykollensvendsen@gmail.com> wrote:


> > +QT6MULTIMEDIA_LICENSE = \
> > +       LGPL-3.0 or GPL-2.0 or GPL-3.0, \
> > +       LGPL-2.1+, \
> > +       BSD-3-Clause (docs), \
> > +       GFDL-1.3 no invariants (docs), \
> > +       BSD-3-Clause (examples), \
> > +       BSL-1.0, \
> > +       MPL-2.0 \
> > +       MIT  
> 
> Thomas, should we order these licenses alpabetically, and avoid
> duplicating the lines?:
> 
> To avoid duplicating lines I would do:
> 
> QT6MULTIMEDIA_LICENSE = \
>       LGPL-3.0 or GPL-2.0 or GPL-3.0, \
>       LGPL-2.1+, \
>       BSD-3-Clause (docs, examples), \
>       GFDL-1.3 no invariants (docs), \
>       BSL-1.0, \
>       MPL-2.0 \
>       MIT
> 
> And order alphabetically:
>       BSD-3-Clause (docs, examples), \
>       BSL-1.0, \
>       GFDL-1.3 no invariants (docs), \
>       LGPL-2.1+, \
>       LGPL-3.0 or GPL-2.0 or GPL-3.0, \
>       MIT \
>       MPL-2.0

Yes, I agree this looks nicer. We don't enforce alphabetic ordering for
the licenses currently, but here it definitely makes sense to do so.


> > +ifeq ($(BR2_PACKAGE_QT6DECLARATIVE),y)
> > +QT6MULTIMEDIA_DEPENDENCIES += qt6declarative
> > +endif  
> 
> I think it is custom to order these ifeq-statements alphabetically?

Yes, it's good to do so.

> > +ifeq ($(BR2_PACKAGE_PULSEAUDIO),y)
> > +QT6MULTIMEDIA_DEPENDENCIES += pulseaudio
> > +endif
> > +
> > +ifeq ($(BR2_PACKAGE_FFMPEG)$(BR2_PACKAGE_PULSEAUDIO),yy)  
> 
> It may be better to avoid doing the BR2_PACKAGE_PULSEAUDIO==y twice,
> by moving the BR2_PACKAGE_FFMPEG==y inside ifeq
> ($(BR2_PACKAGE_PULSEAUDIO),y)-block above.
> 
> > +QT6MULTIMEDIA_CONF_OPTS += -DFFMPEG_DIR=$(BUILD_DIR)/ffmpeg-$(FFMPEG_VERSION) -DQT_DEPLOY_FFMPEG=ON  
> 
> What about splitting the long line to increase readability?:
> 
> QT6MULTIMEDIA_CONF_OPTS += \
>       -DFFMPEG_DIR=$(BUILD_DIR)/ffmpeg-$(FFMPEG_VERSION) \

I don't really like this. Why is this needed? Why does qt6multimedia
need to poke directly into the ffmpeg source code?

If this hadn't been present, I would have applied the patch with the
fixes proposed by Roy, but I'm not a big fan of this ffmpeg thing.

Also, are there some options to explicitly disable things?

I.e, instead of:

+ifeq ($(BR2_PACKAGE_PULSEAUDIO),y)
+QT6MULTIMEDIA_DEPENDENCIES += pulseaudio
+endif

have something to explicitly enable pulseaudio support in
qt6multimedia, and in an "else" clause, something to explicitly disable
pulseaudio support?

Roy: as Baxiche didn't come back with a new iteration, I'm not sure
they will ever come back with a new version. I'm going to mark this
patch as Changes Requested, but if Baxiche doesn't come back with a new
version and no-one else picks it up, it means it will be forgotten.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [Buildroot] [PATCH 1/1] package/qt6: add qt6 multimedia
  2024-09-14 10:13   ` Thomas Petazzoni via buildroot
@ 2024-09-15 17:05     ` baxiche su
  0 siblings, 0 replies; 4+ messages in thread
From: baxiche su @ 2024-09-15 17:05 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Jesse Van Gavere, Samuel Martin, Roy Kollen Svendsen, buildroot


[-- Attachment #1.1: Type: text/plain, Size: 4579 bytes --]

Hi Thomas,

I have a few questions regarding your comments:

1. Should I push a new patch after discussing the patch details with you
via email,
    or should I push a new patch each time we have a discussion?

2. Regarding FFmpeg integration in Qt6 Multimedia compilation:
    When building Qt6 Multimedia from source, it requires specifying the
FFmpeg directory
    containing the include, lib, and bin subdirectories. Are there
alternative methods
    to achieve this integration without directly modifying the FFmpeg
source code?

3. Regarding PulseAudio in Qt6 Multimedia:
    Qt6 Multimedia can be built without PulseAudio. The configuration
process automatically detects
    the PulseAudio library if present. Therefore, there is no need to
specify an alternative option in the absence of PulseAudio.

For more information on Qt6 Multimedia, please see the following resources:
https://doc.qt.io/qt-6/qtmultimedia-building-from-source.html

That's my question about your comment.

By the way, this is my first patch in the open-source community, your
feedback means a lot to me.

Thank you for taking the time to review my work.

Baxiche

Thomas Petazzoni <thomas.petazzoni@bootlin.com> 於 2024年9月14日 週六 下午6:13寫道：

> Hello Baxiche, Hello Roy,
>
> First of all, thanks Roy for the review, much appreciated! See my
> comments below.
>
> On Wed, 14 Aug 2024 13:59:04 +0200
> Roy Kollen Svendsen <roykollensvendsen@gmail.com> wrote:
>
>
> > > +QT6MULTIMEDIA_LICENSE = \
> > > +       LGPL-3.0 or GPL-2.0 or GPL-3.0, \
> > > +       LGPL-2.1+, \
> > > +       BSD-3-Clause (docs), \
> > > +       GFDL-1.3 no invariants (docs), \
> > > +       BSD-3-Clause (examples), \
> > > +       BSL-1.0, \
> > > +       MPL-2.0 \
> > > +       MIT
> >
> > Thomas, should we order these licenses alpabetically, and avoid
> > duplicating the lines?:
> >
> > To avoid duplicating lines I would do:
> >
> > QT6MULTIMEDIA_LICENSE = \
> >       LGPL-3.0 or GPL-2.0 or GPL-3.0, \
> >       LGPL-2.1+, \
> >       BSD-3-Clause (docs, examples), \
> >       GFDL-1.3 no invariants (docs), \
> >       BSL-1.0, \
> >       MPL-2.0 \
> >       MIT
> >
> > And order alphabetically:
> >       BSD-3-Clause (docs, examples), \
> >       BSL-1.0, \
> >       GFDL-1.3 no invariants (docs), \
> >       LGPL-2.1+, \
> >       LGPL-3.0 or GPL-2.0 or GPL-3.0, \
> >       MIT \
> >       MPL-2.0
>
> Yes, I agree this looks nicer. We don't enforce alphabetic ordering for
> the licenses currently, but here it definitely makes sense to do so.
>
>
> > > +ifeq ($(BR2_PACKAGE_QT6DECLARATIVE),y)
> > > +QT6MULTIMEDIA_DEPENDENCIES += qt6declarative
> > > +endif
> >
> > I think it is custom to order these ifeq-statements alphabetically?
>
> Yes, it's good to do so.
>
> > > +ifeq ($(BR2_PACKAGE_PULSEAUDIO),y)
> > > +QT6MULTIMEDIA_DEPENDENCIES += pulseaudio
> > > +endif
> > > +
> > > +ifeq ($(BR2_PACKAGE_FFMPEG)$(BR2_PACKAGE_PULSEAUDIO),yy)
> >
> > It may be better to avoid doing the BR2_PACKAGE_PULSEAUDIO==y twice,
> > by moving the BR2_PACKAGE_FFMPEG==y inside ifeq
> > ($(BR2_PACKAGE_PULSEAUDIO),y)-block above.
> >
> > > +QT6MULTIMEDIA_CONF_OPTS +=
> -DFFMPEG_DIR=$(BUILD_DIR)/ffmpeg-$(FFMPEG_VERSION) -DQT_DEPLOY_FFMPEG=ON
> >
> > What about splitting the long line to increase readability?:
> >
> > QT6MULTIMEDIA_CONF_OPTS += \
> >       -DFFMPEG_DIR=$(BUILD_DIR)/ffmpeg-$(FFMPEG_VERSION) \
>
> I don't really like this. Why is this needed? Why does qt6multimedia
> need to poke directly into the ffmpeg source code?
>
> If this hadn't been present, I would have applied the patch with the
> fixes proposed by Roy, but I'm not a big fan of this ffmpeg thing.
>
> Also, are there some options to explicitly disable things?
>
> I.e, instead of:
>
> +ifeq ($(BR2_PACKAGE_PULSEAUDIO),y)
> +QT6MULTIMEDIA_DEPENDENCIES += pulseaudio
> +endif
>
> have something to explicitly enable pulseaudio support in
> qt6multimedia, and in an "else" clause, something to explicitly disable
> pulseaudio support?
>
> Roy: as Baxiche didn't come back with a new iteration, I'm not sure
> they will ever come back with a new version. I'm going to mark this
> patch as Changes Requested, but if Baxiche doesn't come back with a new
> version and no-one else picks it up, it means it will be forgotten.
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
>

[-- Attachment #1.2: Type: text/html, Size: 6300 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-09-15 17:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-10 17:13 [Buildroot] [PATCH 1/1] package/qt6: add qt6 multimedia baxiche
2024-08-14 11:59 ` Roy Kollen Svendsen
2024-09-14 10:13   ` Thomas Petazzoni via buildroot
2024-09-15 17:05     ` baxiche su

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox