From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bernd Kuhls Date: Sat, 29 Apr 2017 14:29:04 +0200 Subject: [Buildroot] [PATCH v7 08/31] package/kodi: bump to version 17.1-Krypton References: <20170429083751.19625-1-bernd.kuhls@t-online.de> <20170429083751.19625-9-bernd.kuhls@t-online.de> <20170429115608.0b9bf581@free-electrons.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: buildroot@busybox.net Hi Thomas, thanks for the commit! Am Sat, 29 Apr 2017 11:56:08 +0200 schrieb Thomas Petazzoni: > Hello, > > I've applied, after doing a number of minor tweaks. Also, there are a > few things that don't look completely good, and that I believe should be > addressed as follow-up patches. > > On Sat, 29 Apr 2017 10:37:28 +0200, Bernd Kuhls wrote: > >> +config BR2_PACKAGE_KODI_RTMPDUMP >> + bool "kodi rtmp" > > This prompt was quite useless, so I've changed it to "kodi rtmp support > removed". Ok. >> -comment "kodi needs an OpenGL or an openGL ES and EGL backend" >> - depends on BR2_i386 || BR2_x86_64 >> - depends on !BR2_PACKAGE_KODI_GL && !BR2_PACKAGE_KODI_EGL_GLES >> + depends on !BR2_PACKAGE_RPI_USERLAND # rpi depends on gles > > I don't understand why you have this !BR2_PACKAGE_RPI_USERLAND > dependency here. Can you explain? We need to rule out that a rbpi build is done with opengl/egl, otherwise the build will fail like this: http://forum.kodi.tv/showthread.php?tid=305752&pid=2569701#pid2569701 >> config BR2_PACKAGE_KODI_LIBVA >> bool "va" >> + depends on !BR2_PACKAGE_KODI_EGL_GLES > > What about using > > depends on BR2_PACKAGE_KODI_EGL_GL > > here ? > >> + depends on BR2_PACKAGE_XORG7 >> select BR2_PACKAGE_LIBVA >> help >> Enable libva support. >> >> +comment "libva support needs X.org with an openGL backend" >> + depends on !BR2_PACKAGE_XORG7 || BR2_PACKAGE_KODI_EGL_GLES > > And ... || !BR2_PACKAGE_KODI_EGL_GL here ? > >> config BR2_PACKAGE_KODI_LIBVDPAU >> bool "vdpau" >> + depends on !BR2_PACKAGE_KODI_EGL_GLES > > Same suggestion. Ok, sent patch: http://patchwork.ozlabs.org/patch/756733/ >> +KODI_SUBDIR = project/cmake >> + >> +KODI_LIBDVDCSS_VERSION = 2f12236 >> +KODI_LIBDVDNAV_VERSION = 981488f >> +KODI_LIBDVDREAD_VERSION = 17d99db >> + >> +KODI_EXTRA_DOWNLOADS = \ >> + https://github.com/xbmc/libdvdcss/archive/ $(KODI_LIBDVDCSS_VERSION).tar.gz \ >> + https://github.com/xbmc/libdvdnav/archive/ $(KODI_LIBDVDNAV_VERSION).tar.gz \ >> + https://github.com/xbmc/libdvdread/archive/ $(KODI_LIBDVDREAD_VERSION).tar.gz > > Can I say again how much I hate this stuff? > >> +KODI_CONF_OPTS += \ >> + -DLIBDVDCSS_URL=$(BR2_DL_DIR)/$(KODI_LIBDVDCSS_VERSION).tar.gz \ >> + -DLIBDVDNAV_URL=$(BR2_DL_DIR)/$(KODI_LIBDVDNAV_VERSION).tar.gz \ >> + -DLIBDVDREAD_URL=$(BR2_DL_DIR)/$(KODI_LIBDVDREAD_VERSION).tar.gz > > I've moved these below, with the other unconditional CONF_OPTS. Ok. > >> +ifeq ($(BR2_X86_CPU_HAS_SSE),y) >> +KODI_CONF_OPTS += -D_SSE_OK=ON -D_SSE_TRUE=ON > > This looks a bit fishy. Why each option has a _OK option and a _TRUE > option? Is setting both really needed? Yes, otherwise host CPU features sneak into the build parameters, upstream does something similar: https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/modules/ FindSSE.cmake#L81 >> ifeq ($(BR2_PACKAGE_RPI_USERLAND),y) >> +KODI_CONF_OPTS += -DCORE_SYSTEM_NAME=rbpi >> KODI_DEPENDENCIES += rpi-userland >> -KODI_CONF_OPTS += --with-platform=raspberry-pi --enable- player=omxplayer >> -KODI_INCLUDES += \ >> - -I$(STAGING_DIR)/usr/include/interface/vcos/pthreads \ >> - -I$(STAGING_DIR)/usr/include/interface/vmcs_host/linux >> -KODI_LIBS = -lvcos -lvchostif >> -endif >> - >> -ifeq ($(BR2_PACKAGE_HAS_UDEV),y) >> -KODI_DEPENDENCIES += udev >> -KODI_CONF_OPTS += --enable-udev >> else >> -KODI_CONF_OPTS += --disable-udev >> +# these options only exist on non-rbpi systems > > This looks very very weird. Why is LDGOLD related to RPi ? RPi and Linux are considered different platforms, both have their own ArchSetup.cmake: https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/scripts/linux/ ArchSetup.cmake https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/scripts/rbpi/ ArchSetup.cmake LDGOLD is only used for the linux platform: https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/scripts/linux/ ArchSetup.cmake#L34 If you think that treating Linux and RPi as separate platforms is weird then you are not alone, but it may change in Kodi 18: https://github.com/xbmc/xbmc/pull/12021 ;) >> +KODI_CONF_OPTS += -DENABLE_LDGOLD=OFF >> +ifeq ($(BR2_PACKAGE_LIBAMCODEC),y) > > Why should these options be under a "else" of the RPi stuff ? Because all these options do not exist in https://github.com/xbmc/xbmc/ blob/Krypton/project/cmake/scripts/rbpi/ArchSetup.cmake >> -ifeq ($(BR2_PACKAGE_KODI_GL),y) >> -KODI_DEPENDENCIES += libglew libglu libgl xlib_libX11 xlib_libXext \ >> - xlib_libXmu xlib_libXrandr xlib_libXt libdrm >> -KODI_CONF_OPTS += --enable-gl --enable-x11 --disable-gles >> +ifeq ($(BR2_PACKAGE_KODI_GL_EGL),y) >> +KODI_DEPENDENCIES += libegl libglu libgl xlib_libX11 xlib_libXext \ >> + xlib_libXrandr libdrm >> +KODI_CONF_OPTS += -DENABLE_OPENGL=ON -DENABLE_X11=ON - DENABLE_OPENGLES=OFF >> else >> -KODI_CONF_OPTS += --disable-gl --disable-x11 >> +KODI_CONF_OPTS += -DENABLE_OPENGL=OFF -DENABLE_X11=OFF >> ifeq ($(BR2_PACKAGE_KODI_EGL_GLES),y) > > No reason for this being in the "else" of the ifeq > ($(BR2_PACKAGE_KODI_GL_EGL), so I've re-organized this way: > > ifeq ( ... opengl + egl ...) > enable Opengl stuff > else > disable Opengl stuff > endif > > ifeq ( ... opengles + egl ...) > enable OpenglES stuff > else > disable OpenglES stuff > endif Ok. >> +KODI_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) ` $(PKG_CONFIG_HOST_BINARY) --cflags --libs egl`" >> +KODI_CONF_OPTS += -DCMAKE_C_FLAGS="$(TARGET_CFLAGS) ` $(PKG_CONFIG_HOST_BINARY) --cflags --libs egl`" >> +KODI_CONF_OPTS += -DENABLE_OPENGLES=ON > > Reorganized as one assignment to KODI_CONF_OPTS, i.e: > > KODI_CONF_OPTS += \ > ... \ > ... \ > ... Ok. >> + $(HOST_DIR)/usr/bin/xml ed -L \ >> + -d "/addons/addon[text()='service.xbmc.versioncheck']" \ >> + $(KODI_ADDON_MANIFEST) > > So this thing is the only reason why we need xmlstarlet built for the > host system? No, I plan to add more patches to control the skin addon(s) being used, also the default skin addon can then be changed using xmlstarlet: http://lists.busybox.net/pipermail/buildroot/2017-April/189753.html I removed these patches from v6+ because Yann had problems with them: http://lists.busybox.net/pipermail/buildroot/2017-April/190190.html and, as feature patches, they should not block the version bump. Regards, Bernd