diff mbox

[v3,01/38] package/libdvdcss: add Kodi-specific patches

Message ID 20170204114451.20935-2-bernd.kuhls@t-online.de
State Changes Requested
Headers show

Commit Message

Bernd Kuhls Feb. 4, 2017, 11:44 a.m. UTC
The Kodi build system needs .a files to create
usr/lib/kodi/system/players/VideoPlayer/libdvdcss-i486-linux.so

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
 ...css-better-handle-partial-read-in-libc_re.patch | 49 ++++++++++++++++++++++
 ...opy-value-psz_cache-to-dvdcss-psz_cachefi.patch | 33 +++++++++++++++
 package/libdvdcss/libdvdcss.mk                     |  5 +++
 3 files changed, 87 insertions(+)
 create mode 100644 package/libdvdcss/0001-xbmc-libdvdcss-better-handle-partial-read-in-libc_re.patch
 create mode 100644 package/libdvdcss/0002-libdvdcss-Copy-value-psz_cache-to-dvdcss-psz_cachefi.patch

Comments

Yann E. MORIN Feb. 4, 2017, 11:08 p.m. UTC | #1
Bernd, All,

On 2017-02-04 12:44 +0100, Bernd Kuhls spake thusly:
> The Kodi build system needs .a files to create
> usr/lib/kodi/system/players/VideoPlayer/libdvdcss-i486-linux.so

This is really nasty.

What happens if one does not have the .a library, but just the .so one
instead? Does the build really fail?

I've had a quick look at cmake/modules/FindLibDvd.cmake and it only
requires the .a files in the case that it compiles its own version.
Otherwise, it uses the standard find_library() :

    18       find_library(DVDCSS_LIBRARY NAMES dvdcss libdvdcss PATHS ${PC_DVD_libdvdcss_LIBDIR})

which to me does not require that the library be a static one.

So, could you double check if it really does not work with a shared
version?

Ditto for libdvdnav and libdvdread.

Regards,
Yann E. MORIN.

> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>  ...css-better-handle-partial-read-in-libc_re.patch | 49 ++++++++++++++++++++++
>  ...opy-value-psz_cache-to-dvdcss-psz_cachefi.patch | 33 +++++++++++++++
>  package/libdvdcss/libdvdcss.mk                     |  5 +++
>  3 files changed, 87 insertions(+)
>  create mode 100644 package/libdvdcss/0001-xbmc-libdvdcss-better-handle-partial-read-in-libc_re.patch
>  create mode 100644 package/libdvdcss/0002-libdvdcss-Copy-value-psz_cache-to-dvdcss-psz_cachefi.patch
> 
> diff --git a/package/libdvdcss/0001-xbmc-libdvdcss-better-handle-partial-read-in-libc_re.patch b/package/libdvdcss/0001-xbmc-libdvdcss-better-handle-partial-read-in-libc_re.patch
> new file mode 100644
> index 000000000..4d9820642
> --- /dev/null
> +++ b/package/libdvdcss/0001-xbmc-libdvdcss-better-handle-partial-read-in-libc_re.patch
> @@ -0,0 +1,49 @@
> +From d113ac14b45961f958f4aa927c66b3c367f4637c Mon Sep 17 00:00:00 2001
> +From: Voyager1 <voyager@xbmc.org>
> +Date: Sat, 13 Feb 2016 20:44:21 +0100
> +Subject: [PATCH 1/2] [xbmc] [libdvdcss] better handle partial read in
> + libc_read
> +
> +Downloaded from
> +https://github.com/xbmc/libdvdcss/commit/d113ac14b45961f958f4aa927c66b3c367f4637c
> +
> +Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> +---
> + src/device.c | 20 ++++++++++++++------
> + 1 file changed, 14 insertions(+), 6 deletions(-)
> +
> +diff --git a/src/device.c b/src/device.c
> +index af735e0..1936b44 100644
> +--- a/src/device.c
> ++++ b/src/device.c
> +@@ -608,13 +608,21 @@ static int libc_read ( dvdcss_t dvdcss, void *p_buffer, int i_blocks )
> +     off_t i_size, i_ret, i_ret_blocks;
> + 
> +     i_size = (off_t)i_blocks * (off_t)DVDCSS_BLOCK_SIZE;
> +-    i_ret = read( dvdcss->i_fd, p_buffer, i_size );
> +-
> +-    if( i_ret < 0 )
> +-    {
> +-        print_error( dvdcss, "read error" );
> ++    i_ret = 0;
> ++    while (i_ret < i_size)
> ++    {
> ++      off_t i_r;
> ++      i_r = read(dvdcss->i_fd, ((char*)p_buffer) + i_ret, i_size - i_ret);
> ++      if (i_r < 0)
> ++      {
> ++        print_error(dvdcss, "read error");
> +         dvdcss->i_pos = -1;
> +-        return i_ret;
> ++        return i_r;
> ++      }
> ++      if (i_r == 0)
> ++        break;
> ++
> ++      i_ret += i_r;
> +     }
> + 
> +     i_ret_blocks = i_ret / DVDCSS_BLOCK_SIZE;
> +-- 
> +2.8.1
> +
> diff --git a/package/libdvdcss/0002-libdvdcss-Copy-value-psz_cache-to-dvdcss-psz_cachefi.patch b/package/libdvdcss/0002-libdvdcss-Copy-value-psz_cache-to-dvdcss-psz_cachefi.patch
> new file mode 100644
> index 000000000..4251eaf46
> --- /dev/null
> +++ b/package/libdvdcss/0002-libdvdcss-Copy-value-psz_cache-to-dvdcss-psz_cachefi.patch
> @@ -0,0 +1,33 @@
> +From 2f12236bc1c92f73c21e973363f79eb300de603f Mon Sep 17 00:00:00 2001
> +From: Anton Fedchin <anightik@gmail.com>
> +Date: Mon, 15 Feb 2016 16:09:35 +0300
> +Subject: [PATCH 2/2] [libdvdcss] Copy value psz_cache to dvdcss->psz_cachefile
> + if it exists.
> +
> +Downloaded from
> +https://github.com/xbmc/libdvdcss/commit/2f12236bc1c92f73c21e973363f79eb300de603f
> +
> +Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> +---
> + src/libdvdcss.c | 5 +++++
> + 1 file changed, 5 insertions(+)
> +
> +diff --git a/src/libdvdcss.c b/src/libdvdcss.c
> +index 2f78b78..d09d2b3 100644
> +--- a/src/libdvdcss.c
> ++++ b/src/libdvdcss.c
> +@@ -274,6 +274,11 @@ static int set_cache_directory( dvdcss_t dvdcss )
> +         }
> + #endif /* ! defined( _WIN32 ) */
> +     }
> ++    else
> ++    {
> ++      snprintf( dvdcss->psz_cachefile, PATH_MAX, "%s", psz_cache );
> ++      dvdcss->psz_cachefile[PATH_MAX - 1] = '\0';
> ++    }
> + 
> +     /* Check that there is enough space for the cache directory path and the
> +      * block filename. The +1s are path separators. */
> +-- 
> +2.8.1
> +
> diff --git a/package/libdvdcss/libdvdcss.mk b/package/libdvdcss/libdvdcss.mk
> index 8e1c92995..6b13f3f58 100644
> --- a/package/libdvdcss/libdvdcss.mk
> +++ b/package/libdvdcss/libdvdcss.mk
> @@ -11,4 +11,9 @@ LIBDVDCSS_INSTALL_STAGING = YES
>  LIBDVDCSS_LICENSE = GPLv2+
>  LIBDVDCSS_LICENSE_FILES = COPYING
>  
> +ifeq ($(BR2_PACKAGE_KODI),y)
> +LIBDVDCSS_CONF_ENV = CFLAGS="$(TARGET_CFLAGS) -fPIC"
> +LIBDVDCSS_CONF_OPTS = --enable-static
> +endif
> +
>  $(eval $(autotools-package))
> -- 
> 2.11.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Bernd Kuhls Feb. 5, 2017, 7:59 a.m. UTC | #2
[posted and mailed]

Hi Yann,

"Yann E. MORIN" <yann.morin.1998@free.fr> wrote in 
news:20170204230821.GA3805@free.fr:

> Bernd, All,
> 
> On 2017-02-04 12:44 +0100, Bernd Kuhls spake thusly:
>> The Kodi build system needs .a files to create
>> usr/lib/kodi/system/players/VideoPlayer/libdvdcss-i486-linux.so
> 
> This is really nasty.
> 
> What happens if one does not have the .a library, but just the .so one
> instead? Does the build really fail?

Yes.

> I've had a quick look at cmake/modules/FindLibDvd.cmake and it only
> requires the .a files in the case that it compiles its own version.

Kodi 17 still uses autoconf, support for it will be dropped in Kodi 18.

> So, could you double check if it really does not work with a shared
> version?
> 
> Ditto for libdvdnav and libdvdread.

libdvd* needs .a files:
https://github.com/xbmc/xbmc/blob/Krypton/lib/libdvd/Makefile.in#L7

to build wrapped .so libs based on .o files extracted from .a files:

https://github.com/xbmc/xbmc/blob/Krypton/lib/libdvd/Makefile.in#L75
https://github.com/xbmc/xbmc/blob/Krypton/lib/libdvd/Makefile.in#L84
https://github.com/xbmc/xbmc/blob/Krypton/lib/libdvd/Makefile.in#L85

Regards, Bernd
Yann E. MORIN Feb. 5, 2017, 9:24 p.m. UTC | #3
Bernd, All,

On 2017-02-05 08:59 +0100, Bernd Kuhls spake thusly:
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote in 
> news:20170204230821.GA3805@free.fr:
> 
> > Bernd, All,
> > 
> > On 2017-02-04 12:44 +0100, Bernd Kuhls spake thusly:
> >> The Kodi build system needs .a files to create
> >> usr/lib/kodi/system/players/VideoPlayer/libdvdcss-i486-linux.so
> > 
> > This is really nasty.
> > 
> > What happens if one does not have the .a library, but just the .so one
> > instead? Does the build really fail?
> 
> Yes.
> 
> > I've had a quick look at cmake/modules/FindLibDvd.cmake and it only
> > requires the .a files in the case that it compiles its own version.
> 
> Kodi 17 still uses autoconf, support for it will be dropped in Kodi 18.

Ah, OK, I was mislead then.

Indeed their code to handle the libdvdcss, libdvdnav and libdvdread is
just ugly. Han me my flamethrower...

Instead, I believe their code could be simplified (and fixed) using
this:

   73 $(SYSDIR)/libdvdcss-$(ARCH).so: $(WRAPPER) $(WRAPPER_DEF)
   74     [ -d libdvdcss ] || mkdir libdvdcss
   75     cd libdvdcss; $(AR) x $(DVDCSS_A)
   76     $(CC) -o $@ $(SO_LDFLAGS) -Wl,--soname,$@ -Wl,--unresolved-symbols=ignore-all -ldvdcss -lm \
   77         `cat $(WRAPPER_DEF)` $(WRAPPER)

Or something similar...

Ditto for the other lib.

Since this code is slated for removal upstream, I think it is acceptable
that we carry that kind of patch, especially since the replacement
CMakeList.txt looks like it is sane.

Could you test this, please?

Regards,
Yann E. MORIN.
Bernd Kuhls Feb. 6, 2017, 7:25 p.m. UTC | #4
Hi Yann,

Am Sun, 05 Feb 2017 22:24:09 +0100 schrieb Yann E. MORIN:

> Instead, I believe their code could be simplified (and fixed) using
> this:
> 
>    73 $(SYSDIR)/libdvdcss-$(ARCH).so: $(WRAPPER) $(WRAPPER_DEF)
>    74     [ -d libdvdcss ] || mkdir libdvdcss
>    75     cd libdvdcss; $(AR) x $(DVDCSS_A)

afaics line 75 still needs libdvdcss.a because thats the content of $(DVDCSS_A):
https://github.com/xbmc/xbmc/blob/Krypton/lib/libdvd/Makefile.in#L11

> Could you test this, please?

I will try, but I am unsure that your proposal will work.

Regards, Bernd
Bernd Kuhls Feb. 6, 2017, 9:15 p.m. UTC | #5
Am Mon, 06 Feb 2017 20:25:38 +0100 schrieb Bernd Kuhls:

> Hi Yann,
> 
> Am Sun, 05 Feb 2017 22:24:09 +0100 schrieb Yann E. MORIN:
> 
>> Instead, I believe their code could be simplified (and fixed) using
>> this:
>> 
>>    73 $(SYSDIR)/libdvdcss-$(ARCH).so: $(WRAPPER) $(WRAPPER_DEF)
>>    74     [ -d libdvdcss ] || mkdir libdvdcss
>>    75     cd libdvdcss; $(AR) x $(DVDCSS_A)
> 
> afaics line 75 still needs libdvdcss.a because thats the content of $(DVDCSS_A):
> https://github.com/xbmc/xbmc/blob/Krypton/lib/libdvd/Makefile.in#L11
> 
>> Could you test this, please?
> 
> I will try, but I am unsure that your proposal will work.

Hi Yann,

tried and failed.

Regards, Bernd
Thomas Petazzoni Feb. 9, 2017, 10:29 p.m. UTC | #6
Hello,

On Mon, 06 Feb 2017 22:15:39 +0100, Bernd Kuhls wrote:

> > I will try, but I am unsure that your proposal will work.  
> 
> Hi Yann,
> 
> tried and failed.

Are these .a files still needed after the switch to CMake as the build
system for Kodi?

I would be OK to merge those hacks for libdvdcss, libdvdread and
libdvdnav if we know they are going to be removed when the next version
of Kodi is released. However, if these hacks are still needed for the
next version of Kodi, then I'm afraid I won't merge them. This is way
too ugly, and what Kodi is doing here is truly horrible and should be
fixed in Kodi.

This is a quite critical topic, as it's blocking the inclusion of the
new version of Kodi in Buildroot.

Also, for the patches that apply to the libdvdcss, libdvdread and
libdvdnav code base, it would be good to know if they have been
submitted to the respective upstream projects, or if it's stuff that we
may have to keep in Buildroot forever.

Thanks!

Thomas
Bernd Kuhls Feb. 11, 2017, 4:03 p.m. UTC | #7
[posted and mailed]

Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote
in news:20170209232937.2a071f11@free-electrons.com: 

Hi Thomas,

> Are these .a files still needed after the switch to CMake as the build
> system for Kodi?

fortunately no.

> Also, for the patches that apply to the libdvdcss, libdvdread and
> libdvdnav code base, it would be good to know if they have been
> submitted to the respective upstream projects, or if it's stuff that we
> may have to keep in Buildroot forever.

Afaics when using the new CMake-based build system of Kodi 17 it will
build libdvd* itself by using Kodi-specific tarballs:

https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/modules/FindLibDvd.
cmake#L100
https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/modules/FindLibDvd.
cmake#L132
https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/modules/FindLibDvd.
cmake#L164

So we do neither need to patch libdvd* anymore nor provide them as 
dependent packages, KODI_EXTRA_DOWNLOADS can be used to provide the 
tarballs.

First build tests were successful but I would like to be cautious about the 
switch to CMake because I only started to use it today with an x86_64 
toolchain so I have not much experience with it, as of this moment I never 
did a runtime test of the compiled binaries. I hope to find some time for 
this in the coming days but until then you can find my patches here:

https://github.com/bkuhls/buildroot/tree/kodi_cmake

Please note that LibreElec and Gentoo already build Kodi 17 using CMake:

https://github.com/LibreELEC/LibreELEC.tv/blob/libreelec-
8.0/packages/mediacenter/kodi/package.mk#L34

https://gitweb.gentoo.org/repo/gentoo.git/tree/media-tv/kodi/kodi-
17.0.ebuild#n161

Do we still have external toolchains based on uClibc 0.9.33.2?
If yes, we need to keep 
https://git.buildroot.net/buildroot/tree/package/kodi/0003-ALSA-fix-device-
change-event-support.patch and add CMake checks for eventfd_read & 
eventfd_write.

Regards, Bernd
Yann E. MORIN Feb. 11, 2017, 4:37 p.m. UTC | #8
Bernd, All,

On 2017-02-11 17:03 +0100, Bernd Kuhls spake thusly:
> Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote
> in news:20170209232937.2a071f11@free-electrons.com: 
> 
> Hi Thomas,
> 
> > Are these .a files still needed after the switch to CMake as the build
> > system for Kodi?
> 
> fortunately no.
> 
> > Also, for the patches that apply to the libdvdcss, libdvdread and
> > libdvdnav code base, it would be good to know if they have been
> > submitted to the respective upstream projects, or if it's stuff that we
> > may have to keep in Buildroot forever.
> 
> Afaics when using the new CMake-based build system of Kodi 17 it will
> build libdvd* itself by using Kodi-specific tarballs:
> 
> https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/modules/FindLibDvd.
> cmake#L100
> https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/modules/FindLibDvd.
> cmake#L132
> https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/modules/FindLibDvd.
> cmake#L164

That's just insane that they got rid of their internal copies, to just
re-introduce them a release later... :-(

However, from those files you quoted:

    https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/modules/FindLibDvd.cmake#L2

So Kodi actually *can* use external dvdcss, dvdread and dvdnav
libraries. It /just/ needs KODI_DEPENDSBUILD (whjatever that is supposed
to mean... :-/ )

> So we do neither need to patch libdvd* anymore nor provide them as 
> dependent packages, KODI_EXTRA_DOWNLOADS can be used to provide the 
> tarballs.
> 
> First build tests were successful but I would like to be cautious about the 
> switch to CMake because I only started to use it today with an x86_64 
> toolchain so I have not much experience with it, as of this moment I never 
> did a runtime test of the compiled binaries. I hope to find some time for 
> this in the coming days but until then you can find my patches here:
> 
> https://github.com/bkuhls/buildroot/tree/kodi_cmake
> 
> Please note that LibreElec and Gentoo already build Kodi 17 using CMake:
> 
> https://github.com/LibreELEC/LibreELEC.tv/blob/libreelec-
> 8.0/packages/mediacenter/kodi/package.mk#L34
> 
> https://gitweb.gentoo.org/repo/gentoo.git/tree/media-tv/kodi/kodi-
> 17.0.ebuild#n161
> 
> Do we still have external toolchains based on uClibc 0.9.33.2?
> If yes, we need to keep 
> https://git.buildroot.net/buildroot/tree/package/kodi/0003-ALSA-fix-device-
> change-event-support.patch and add CMake checks for eventfd_read & 
> eventfd_write.

I'd say we should not care about uClibc < 1.0, it's dead.

So, either we mark Kodi as !uclibc, or we let the user keep the pieces
if they are stuck with such an antediluvian toolchain. Get into the 21st
century, man... ;-]

Actually, I think we should start detecting such old toolchains and
refuse to use them.

Regards,
Yann E. MORIN.
Bernd Kuhls Feb. 11, 2017, 5:02 p.m. UTC | #9
[posted and mailed]

Hi Yann,

"Yann E. MORIN" <yann.morin.1998@free.fr> wrote in
news:20170211163706.GB20146@free.fr: 

> So Kodi actually *can* use external dvdcss, dvdread and dvdnav
> libraries. It /just/ needs KODI_DEPENDSBUILD (whjatever that is supposed
> to mean... :-/ )

Kodi can build _all_ its dependencies during build:
https://github.com/xbmc/xbmc/tree/Krypton/tools/depends/target
https://github.com/xbmc/xbmc/blob/Krypton/tools/depends/target/Toolchain.cmak
e.in#L84

If KODI_DEPENDSBUILD = OFF then Kodi assumes all dependencies were build
outside Kodi: https://github.com/xbmc/xbmc/pull/11088

Yes, maybe we could build libdvd* using buildroot, but we would still need
to add the Kodi-specific patches to the packages which I want to avoid.

Regards, Bernd
Yann E. MORIN Feb. 11, 2017, 5:45 p.m. UTC | #10
Bernd, All,

On 2017-02-11 18:02 +0100, Bernd Kuhls spake thusly:
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote in
> news:20170211163706.GB20146@free.fr: 
> 
> > So Kodi actually *can* use external dvdcss, dvdread and dvdnav
> > libraries. It /just/ needs KODI_DEPENDSBUILD (whjatever that is supposed
> > to mean... :-/ )
> 
> Kodi can build _all_ its dependencies during build:
> https://github.com/xbmc/xbmc/tree/Krypton/tools/depends/target
> https://github.com/xbmc/xbmc/blob/Krypton/tools/depends/target/Toolchain.cmak
> e.in#L84
> 
> If KODI_DEPENDSBUILD = OFF then Kodi assumes all dependencies were build
> outside Kodi: https://github.com/xbmc/xbmc/pull/11088

From that cmake file, it seems we need to set KODI_DEPENDSBUILD=ON for
it to use pre-built libs, no?

And that Toolchain.cmake.in file you pointed to above seem to default to
using the externally-provided libs...

Unless I'm completely out of mny shoes... :-/

> Yes, maybe we could build libdvd* using buildroot, but we would still need
> to add the Kodi-specific patches to the packages which I want to avoid.

Well, that's what your series was doing, no?

In the end, I think I'd prefer we (unconditionally) patch the packages,
as long as those patches seem reasonable (i.e. upstreamable).

Of course, if those patches are really Kodi-specific (i.e. introduces
changes just for the sake of Kodi and are not upstreamable), then I'd
revise my position.

Regards,
Yann E. MORIN.
Thomas Petazzoni Feb. 12, 2017, 12:51 p.m. UTC | #11
Hello,

On Sat, 11 Feb 2017 17:03:42 +0100, Bernd Kuhls wrote:

> > Are these .a files still needed after the switch to CMake as the build
> > system for Kodi?  
> 
> fortunately no.

Good.

> > Also, for the patches that apply to the libdvdcss, libdvdread and
> > libdvdnav code base, it would be good to know if they have been
> > submitted to the respective upstream projects, or if it's stuff that we
> > may have to keep in Buildroot forever.  
> 
> Afaics when using the new CMake-based build system of Kodi 17 it will
> build libdvd* itself by using Kodi-specific tarballs:
> 
> https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/modules/FindLibDvd.
> cmake#L100
> https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/modules/FindLibDvd.
> cmake#L132
> https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/modules/FindLibDvd.
> cmake#L164
> 
> So we do neither need to patch libdvd* anymore nor provide them as 
> dependent packages, KODI_EXTRA_DOWNLOADS can be used to provide the 
> tarballs.

Except that we definitely don't want this thing. Kodi should use the
libdvd* libraries built by Buildroot. Not download/build its own.

> First build tests were successful but I would like to be cautious about the 
> switch to CMake because I only started to use it today with an x86_64 
> toolchain so I have not much experience with it, as of this moment I never 
> did a runtime test of the compiled binaries. I hope to find some time for 
> this in the coming days but until then you can find my patches here:
> 
> https://github.com/bkuhls/buildroot/tree/kodi_cmake
> 
> Please note that LibreElec and Gentoo already build Kodi 17 using CMake:
> 
> https://github.com/LibreELEC/LibreELEC.tv/blob/libreelec-
> 8.0/packages/mediacenter/kodi/package.mk#L34
> 
> https://gitweb.gentoo.org/repo/gentoo.git/tree/media-tv/kodi/kodi-
> 17.0.ebuild#n161

I leave it up to you when it is a good time to switch to the CMake
based build system.

> Do we still have external toolchains based on uClibc 0.9.33.2?
> If yes, we need to keep 
> https://git.buildroot.net/buildroot/tree/package/kodi/0003-ALSA-fix-device-
> change-event-support.patch and add CMake checks for eventfd_read & 
> eventfd_write.

No, we don't care about uClibc 0.9.33.2 anymore. The only external
toolchain that was using an old uClibc version was the Analog Devices
Blackfin toolchain, and we got rid of it during the last cycle.

Thanks,

Thomas
Bernd Kuhls Feb. 17, 2017, 6:38 p.m. UTC | #12
[posted and mailed]

Hi Thomas,

Thomas Petazzoni <thomas.petazzoni-
wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote in 
news:20170212135110.61e55c66@free-electrons.com:

> Except that we definitely don't want this thing. Kodi should use the
> libdvd* libraries built by Buildroot. Not download/build its own.

in this case we need to
- apply the Kodi-specific patches, or
- switch libdvd* upstream to the Kodi repos

The packages were all forked by Kodi:

https://github.com/xbmc/libdvdnav/commits/master
https://github.com/xbmc/libdvdread/commits/master
https://github.com/xbmc/libdvdcss/commits/master

to add functions for supporting iso images by their internal VFS 
implementation, for example dvdnav_get_vm:

https://github.com/xbmc/xbmc/search?utf8=%E2%9C%93&q=dvdnav_get_vm

This function is not defined in upstream libdvdnav, but added by
https://github.com/xbmc/libdvdnav/commit/8305696be79fe650d3d9eee29b2a88e020d7
c58f, guarded with ifdef _XBMC...

You can however link kodi.bin successfully with an unpatched libdvdnav, but 
be warned: Kodi will crash during runtime like this:

http://www.vdr-portal.de/board16-video-disk-recorder/board99-
distributionen/board96-yavdr/p1279428-yavdr-0-6-kodi-amazon-prime-instant-
video-addon-kein-ton/#post1279428

10:51:04 T:140165711784000 WARNING: Unable to resolve: /libdvdnav-x86_64-
linux.so dvdnav_get_vm, reason: /usr/lib/x86_64-linux-
gnu/kodi/system/players/VideoPlayer/libdvdnav-x86_64-linux.so: undefined 
symbol: dvdnav_get_vm
10:51:04 T:140165711784000 ERROR: Unable to resolve exports from dll 
special://xbmcbin/system/players/VideoPlayer/libdvdnav-x86_64-linux.so

because libdvd* is wrapped with a dllwrapper:
https://github.com/xbmc/xbmc/blob/Krypton/project/cmake/modules/FindLibDvd.cm
ake#L194

Currently I am using

KODI_LIBDVDCSS_VERSION = 2f12236
KODI_LIBDVDNAV_VERSION = 43b5f81
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
        
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

to provide the libdvd* source to be compiled during Kodi build. With them I 
can play iso dvd images without problems.

> I leave it up to you when it is a good time to switch to the CMake
> based build system.

This week I switched my production system to a CMake-build Kodi created by 
buildroot so the next iteration of my patch series is not far away :)

Regards, Bernd
Yann E. MORIN Feb. 17, 2017, 6:54 p.m. UTC | #13
Bernd, Thomas, All,

On 2017-02-17 19:38 +0100, Bernd Kuhls spake thusly:
> Thomas Petazzoni <thomas.petazzoni-
> wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote in 
> news:20170212135110.61e55c66@free-electrons.com:
> 
> > Except that we definitely don't want this thing. Kodi should use the
> > libdvd* libraries built by Buildroot. Not download/build its own.
> 
> in this case we need to
> - apply the Kodi-specific patches, or
> - switch libdvd* upstream to the Kodi repos

Or:

  - add new pacakges, kodi-libdvdnav, kodi-libdvdread, kodi-libdvdcss

Not that it is much better than having them managed by Kodi.

Except that those managed by Kodi would be downloaded at configure or
build time, not when doing 'make source', so we could not do an off-line
build. That alone would warrant they be separate packages, IMHO.

Of course, this then raises the questions:

 1- can they replace the existing ones for other applications?
 2- would they conflict (filenames) with the pristine versions?

     1 | 2 | Solution
    ---+---+--------------------------------------------
     N | N | separate, unrelated packages with no "depends on !the-others"
     N | Y | dunno...
     Y | N | virtual packages for each, like libjpeg?
     Y | Y | official ones "depends on !the-others"

Regards,
Yann E. MORIN.
Bernd Kuhls Feb. 17, 2017, 7:03 p.m. UTC | #14
Hi Yann,

Am Fri, 17 Feb 2017 19:54:34 +0100 schrieb Yann E. MORIN:

> Except that those managed by Kodi would be downloaded at configure or
> build time, not when doing 'make source', so we could not do an off-line
> build. That alone would warrant they be separate packages, IMHO.

'make source' just downloaded

 https://github.com/xbmc/libdvdcss/archive/2f12236.tar.gz
 https://github.com/xbmc/libdvdnav/archive/43b5f81.tar.gz
 https://github.com/xbmc/libdvdread/archive/17d99db.tar.gz

here. You can find my git tree for testing here:
https://github.com/bkuhls/buildroot/tree/kodi_cmake

By using _EXTRA_DOWNLOADS an offline build should be possible:
https://github.com/bkuhls/buildroot/blob/kodi_cmake/package/kodi/kodi.mk#L124

Regards, Bernd
Yann E. MORIN Feb. 17, 2017, 9:52 p.m. UTC | #15
Bernd,

On 2017-02-17 20:03 +0100, Bernd Kuhls spake thusly:
> Am Fri, 17 Feb 2017 19:54:34 +0100 schrieb Yann E. MORIN:
> 
> > Except that those managed by Kodi would be downloaded at configure or
> > build time, not when doing 'make source', so we could not do an off-line
> > build. That alone would warrant they be separate packages, IMHO.
> 
> 'make source' just downloaded
> 
>  https://github.com/xbmc/libdvdcss/archive/2f12236.tar.gz
>  https://github.com/xbmc/libdvdnav/archive/43b5f81.tar.gz
>  https://github.com/xbmc/libdvdread/archive/17d99db.tar.gz

That's supposing that we do patch the existing libdvd* packages with the
kodi patches.

Which I am not too fond of to start with, hence all the discussion we're
having about it...

But...

If the patched version are still compatible with the unpatched version
(i.e. it offers a superset of the API), then that could probably be
acceptable (and thus forget about that case in my previous table).

Damned, can't the Kodi folks work with upstream to add features rather
than fork? Sigh...

Regards,
Yann E. MORIN.
Bernd Kuhls Feb. 19, 2017, 8:43 a.m. UTC | #16
Am Fri, 17 Feb 2017 22:52:31 +0100 schrieb Yann E. MORIN:

> If the patched version are still compatible with the unpatched version
> (i.e. it offers a superset of the API), then that could probably be
> acceptable (and thus forget about that case in my previous table).

Hi,

I tried to patch the existing buildroot libdvd* packages with the Kodi-
specific stuff and did a successful runtime test, building static libs is 
not necessary anymore with the CMake build system :)

You can find the current patch series here as RFC:
https://github.com/bkuhls/buildroot/commits/kodi_cmake_libdvd

Please note that I am using HEAD of the Krypton branch because one of my 
patches needed for cross-compiling already got merged upstream, the iconv 
one is pending.

Regards, Bernd
diff mbox

Patch

diff --git a/package/libdvdcss/0001-xbmc-libdvdcss-better-handle-partial-read-in-libc_re.patch b/package/libdvdcss/0001-xbmc-libdvdcss-better-handle-partial-read-in-libc_re.patch
new file mode 100644
index 000000000..4d9820642
--- /dev/null
+++ b/package/libdvdcss/0001-xbmc-libdvdcss-better-handle-partial-read-in-libc_re.patch
@@ -0,0 +1,49 @@ 
+From d113ac14b45961f958f4aa927c66b3c367f4637c Mon Sep 17 00:00:00 2001
+From: Voyager1 <voyager@xbmc.org>
+Date: Sat, 13 Feb 2016 20:44:21 +0100
+Subject: [PATCH 1/2] [xbmc] [libdvdcss] better handle partial read in
+ libc_read
+
+Downloaded from
+https://github.com/xbmc/libdvdcss/commit/d113ac14b45961f958f4aa927c66b3c367f4637c
+
+Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
+---
+ src/device.c | 20 ++++++++++++++------
+ 1 file changed, 14 insertions(+), 6 deletions(-)
+
+diff --git a/src/device.c b/src/device.c
+index af735e0..1936b44 100644
+--- a/src/device.c
++++ b/src/device.c
+@@ -608,13 +608,21 @@ static int libc_read ( dvdcss_t dvdcss, void *p_buffer, int i_blocks )
+     off_t i_size, i_ret, i_ret_blocks;
+ 
+     i_size = (off_t)i_blocks * (off_t)DVDCSS_BLOCK_SIZE;
+-    i_ret = read( dvdcss->i_fd, p_buffer, i_size );
+-
+-    if( i_ret < 0 )
+-    {
+-        print_error( dvdcss, "read error" );
++    i_ret = 0;
++    while (i_ret < i_size)
++    {
++      off_t i_r;
++      i_r = read(dvdcss->i_fd, ((char*)p_buffer) + i_ret, i_size - i_ret);
++      if (i_r < 0)
++      {
++        print_error(dvdcss, "read error");
+         dvdcss->i_pos = -1;
+-        return i_ret;
++        return i_r;
++      }
++      if (i_r == 0)
++        break;
++
++      i_ret += i_r;
+     }
+ 
+     i_ret_blocks = i_ret / DVDCSS_BLOCK_SIZE;
+-- 
+2.8.1
+
diff --git a/package/libdvdcss/0002-libdvdcss-Copy-value-psz_cache-to-dvdcss-psz_cachefi.patch b/package/libdvdcss/0002-libdvdcss-Copy-value-psz_cache-to-dvdcss-psz_cachefi.patch
new file mode 100644
index 000000000..4251eaf46
--- /dev/null
+++ b/package/libdvdcss/0002-libdvdcss-Copy-value-psz_cache-to-dvdcss-psz_cachefi.patch
@@ -0,0 +1,33 @@ 
+From 2f12236bc1c92f73c21e973363f79eb300de603f Mon Sep 17 00:00:00 2001
+From: Anton Fedchin <anightik@gmail.com>
+Date: Mon, 15 Feb 2016 16:09:35 +0300
+Subject: [PATCH 2/2] [libdvdcss] Copy value psz_cache to dvdcss->psz_cachefile
+ if it exists.
+
+Downloaded from
+https://github.com/xbmc/libdvdcss/commit/2f12236bc1c92f73c21e973363f79eb300de603f
+
+Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
+---
+ src/libdvdcss.c | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/src/libdvdcss.c b/src/libdvdcss.c
+index 2f78b78..d09d2b3 100644
+--- a/src/libdvdcss.c
++++ b/src/libdvdcss.c
+@@ -274,6 +274,11 @@ static int set_cache_directory( dvdcss_t dvdcss )
+         }
+ #endif /* ! defined( _WIN32 ) */
+     }
++    else
++    {
++      snprintf( dvdcss->psz_cachefile, PATH_MAX, "%s", psz_cache );
++      dvdcss->psz_cachefile[PATH_MAX - 1] = '\0';
++    }
+ 
+     /* Check that there is enough space for the cache directory path and the
+      * block filename. The +1s are path separators. */
+-- 
+2.8.1
+
diff --git a/package/libdvdcss/libdvdcss.mk b/package/libdvdcss/libdvdcss.mk
index 8e1c92995..6b13f3f58 100644
--- a/package/libdvdcss/libdvdcss.mk
+++ b/package/libdvdcss/libdvdcss.mk
@@ -11,4 +11,9 @@  LIBDVDCSS_INSTALL_STAGING = YES
 LIBDVDCSS_LICENSE = GPLv2+
 LIBDVDCSS_LICENSE_FILES = COPYING
 
+ifeq ($(BR2_PACKAGE_KODI),y)
+LIBDVDCSS_CONF_ENV = CFLAGS="$(TARGET_CFLAGS) -fPIC"
+LIBDVDCSS_CONF_OPTS = --enable-static
+endif
+
 $(eval $(autotools-package))