diff mbox

[RFC/PATCH,1/1] gst-fsl-plugins: fix compile for sysroot based on newer linux kernel headers

Message ID 1393366722-27644-2-git-send-email-ps.report@gmx.net
State Superseded
Headers show

Commit Message

Peter Seiderer Feb. 25, 2014, 10:18 p.m. UTC
The v4l2 ioctl VIDIOC_DBG_G_CHIP_IDENT was removed in linux-3.11 (see [1]/[2]).

To avoid build failure of the gst-fsl-plugins/mfw_gst_v4lsrc with toolchains based
on newer linux kernel headers this patch gives the include of the custom linux
kernel headers precedence over the sysroot linux kernel headers (oposed to the
previous behavior).

Fixes the following compile error with buildroot/glibc toolchain:

mfw_gst_v4lsrc.c: In function 'mfw_gst_v4lsrc_capture_setup':
mfw_gst_v4lsrc.c:452:10: error: variable 'chip' has initializer but incomplete type
   struct v4l2_dbg_chip_ident chip = {0};
          ^
mfw_gst_v4lsrc.c:452:10: warning: excess elements in struct initializer [enabled by default]
mfw_gst_v4lsrc.c:452:10: warning: (near initialization for 'chip') [enabled by default]
mfw_gst_v4lsrc.c:452:30: error: storage size of 'chip' isn't known
   struct v4l2_dbg_chip_ident chip = {0};
                              ^
mfw_gst_v4lsrc.c:464:22: error: 'VIDIOC_DBG_G_CHIP_IDENT' undeclared (first use in this function)
   if (ioctl (fd_v4l, VIDIOC_DBG_G_CHIP_IDENT, &chip)) {
                      ^

[1] http://www.spinics.net/lists/linux-media/msg64081.html
[2] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/videodev2.h?id=b71c99801e18eb172ae34851daf25044a3bf644a

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
 package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Petazzoni March 2, 2014, 8:48 a.m. UTC | #1
Arnout,

Since you originally added the package with -idirafter, can you comment
on the below patch? To me it makes sense that if the package needs
special kernel headers, it should use first the headers from the kernel
and then only the sysroot headers. Am I missing something that lead you
to the usage of -idirafter?

See the patch below.

Thanks!

Thomas

On Tue, 25 Feb 2014 23:18:42 +0100, Peter Seiderer wrote:
> The v4l2 ioctl VIDIOC_DBG_G_CHIP_IDENT was removed in linux-3.11 (see [1]/[2]).
> 
> To avoid build failure of the gst-fsl-plugins/mfw_gst_v4lsrc with toolchains based
> on newer linux kernel headers this patch gives the include of the custom linux
> kernel headers precedence over the sysroot linux kernel headers (oposed to the
> previous behavior).
> 
> Fixes the following compile error with buildroot/glibc toolchain:
> 
> mfw_gst_v4lsrc.c: In function 'mfw_gst_v4lsrc_capture_setup':
> mfw_gst_v4lsrc.c:452:10: error: variable 'chip' has initializer but incomplete type
>    struct v4l2_dbg_chip_ident chip = {0};
>           ^
> mfw_gst_v4lsrc.c:452:10: warning: excess elements in struct initializer [enabled by default]
> mfw_gst_v4lsrc.c:452:10: warning: (near initialization for 'chip') [enabled by default]
> mfw_gst_v4lsrc.c:452:30: error: storage size of 'chip' isn't known
>    struct v4l2_dbg_chip_ident chip = {0};
>                               ^
> mfw_gst_v4lsrc.c:464:22: error: 'VIDIOC_DBG_G_CHIP_IDENT' undeclared (first use in this function)
>    if (ioctl (fd_v4l, VIDIOC_DBG_G_CHIP_IDENT, &chip)) {
>                       ^
> 
> [1] http://www.spinics.net/lists/linux-media/msg64081.html
> [2] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/videodev2.h?id=b71c99801e18eb172ae34851daf25044a3bf644a
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> ---
>  package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk b/package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk
> index ccdb521..610bd78 100644
> --- a/package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk
> +++ b/package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk
> @@ -23,7 +23,7 @@ GST_FSL_PLUGINS_CONF_ENV = PLATFORM=$(BR2_PACKAGE_GST_FSL_PLUGINS_PLATFORM)
>  
>  # needs access to imx-specific kernel headers
>  GST_FSL_PLUGINS_DEPENDENCIES += linux
> -GST_FSL_PLUGINS_CONF_ENV += CPPFLAGS="$(TARGET_CPPFLAGS) -idirafter $(LINUX_DIR)/include"
> +GST_FSL_PLUGINS_CONF_ENV += CPPFLAGS="$(TARGET_CPPFLAGS) -I$(LINUX_DIR)/include"
>  
>  ifeq ($(BR2_PACKAGE_XLIB_LIBX11),y)
>  GST_FSL_PLUGINS_DEPENDENCIES += xlib_libX11
Arnout Vandecappelle March 4, 2014, 5:40 p.m. UTC | #2
On 02/03/14 09:48, Thomas Petazzoni wrote:
> Arnout,
> 
> Since you originally added the package with -idirafter, can you comment
> on the below patch? To me it makes sense that if the package needs
> special kernel headers, it should use first the headers from the kernel
> and then only the sysroot headers. Am I missing something that lead you
> to the usage of -idirafter?

 Ow yes good that you remind me...

 The problem is that the kernel headers in /usr/include/linux are not the
same as the ones in $(LINUX_DIR)/include/linux: headers_install does some
fixups on them. Also, $(LINUX_DIR)/include/net is completely different
from /usr/include/net. So putting $(LINUX_DIR)/include in front of the
system dirs is a bad idea. As in, it didn't work for me :-)

 Maybe a generic solution for packages that require specific kernel
headers is to include a headers_install in the kernel's build?

 Anyway, for this one, if it works with -I with all our external
toolchains and with an internal uClibc and glibc toolchain, then I guess
it's OK. Perhaps something got fixed in the gst-fsl-plugins bump.


 Regards,
 Arnout

> 
> See the patch below.
> 
> Thanks!
> 
> Thomas
> 
> On Tue, 25 Feb 2014 23:18:42 +0100, Peter Seiderer wrote:
>> The v4l2 ioctl VIDIOC_DBG_G_CHIP_IDENT was removed in linux-3.11 (see [1]/[2]).
>>
>> To avoid build failure of the gst-fsl-plugins/mfw_gst_v4lsrc with toolchains based
>> on newer linux kernel headers this patch gives the include of the custom linux
>> kernel headers precedence over the sysroot linux kernel headers (oposed to the
>> previous behavior).
>>
>> Fixes the following compile error with buildroot/glibc toolchain:
>>
>> mfw_gst_v4lsrc.c: In function 'mfw_gst_v4lsrc_capture_setup':
>> mfw_gst_v4lsrc.c:452:10: error: variable 'chip' has initializer but incomplete type
>>    struct v4l2_dbg_chip_ident chip = {0};
>>           ^
>> mfw_gst_v4lsrc.c:452:10: warning: excess elements in struct initializer [enabled by default]
>> mfw_gst_v4lsrc.c:452:10: warning: (near initialization for 'chip') [enabled by default]
>> mfw_gst_v4lsrc.c:452:30: error: storage size of 'chip' isn't known
>>    struct v4l2_dbg_chip_ident chip = {0};
>>                               ^
>> mfw_gst_v4lsrc.c:464:22: error: 'VIDIOC_DBG_G_CHIP_IDENT' undeclared (first use in this function)
>>    if (ioctl (fd_v4l, VIDIOC_DBG_G_CHIP_IDENT, &chip)) {
>>                       ^
>>
>> [1] http://www.spinics.net/lists/linux-media/msg64081.html
>> [2] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/videodev2.h?id=b71c99801e18eb172ae34851daf25044a3bf644a
>>
>> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
>> ---
>>  package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk b/package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk
>> index ccdb521..610bd78 100644
>> --- a/package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk
>> +++ b/package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk
>> @@ -23,7 +23,7 @@ GST_FSL_PLUGINS_CONF_ENV = PLATFORM=$(BR2_PACKAGE_GST_FSL_PLUGINS_PLATFORM)
>>  
>>  # needs access to imx-specific kernel headers
>>  GST_FSL_PLUGINS_DEPENDENCIES += linux
>> -GST_FSL_PLUGINS_CONF_ENV += CPPFLAGS="$(TARGET_CPPFLAGS) -idirafter $(LINUX_DIR)/include"
>> +GST_FSL_PLUGINS_CONF_ENV += CPPFLAGS="$(TARGET_CPPFLAGS) -I$(LINUX_DIR)/include"
>>  
>>  ifeq ($(BR2_PACKAGE_XLIB_LIBX11),y)
>>  GST_FSL_PLUGINS_DEPENDENCIES += xlib_libX11
> 
> 
>
Thomas Petazzoni March 4, 2014, 9:50 p.m. UTC | #3
Dear Arnout Vandecappelle,

On Tue, 04 Mar 2014 18:40:53 +0100, Arnout Vandecappelle wrote:

>  The problem is that the kernel headers in /usr/include/linux are not the
> same as the ones in $(LINUX_DIR)/include/linux: headers_install does some
> fixups on them. Also, $(LINUX_DIR)/include/net is completely different
> from /usr/include/net. So putting $(LINUX_DIR)/include in front of the
> system dirs is a bad idea. As in, it didn't work for me :-)

Ok.

>  Maybe a generic solution for packages that require specific kernel
> headers is to include a headers_install in the kernel's build?

But will it work properly? I mean, most of the time, the packages that
need this are vendor-specific packages, that want to access specific
definitions in vendor-specific kernels. And the quality of
vendor-specific kernels is usually quite low, so there are quite some
chances that headers_install might not necessarily do the right thing
as it will not have been properly tested...

>  Anyway, for this one, if it works with -I with all our external
> toolchains and with an internal uClibc and glibc toolchain, then I guess
> it's OK. Perhaps something got fixed in the gst-fsl-plugins bump.

Is this a Acked-by for the patch?

Thomas
Thomas Petazzoni April 19, 2014, 9:56 p.m. UTC | #4
Arnout,

On Tue, 4 Mar 2014 22:50:57 +0100, Thomas Petazzoni wrote:

> >  Maybe a generic solution for packages that require specific kernel
> > headers is to include a headers_install in the kernel's build?
> 
> But will it work properly? I mean, most of the time, the packages that
> need this are vendor-specific packages, that want to access specific
> definitions in vendor-specific kernels. And the quality of
> vendor-specific kernels is usually quite low, so there are quite some
> chances that headers_install might not necessarily do the right thing
> as it will not have been properly tested...
> 
> >  Anyway, for this one, if it works with -I with all our external
> > toolchains and with an internal uClibc and glibc toolchain, then I guess
> > it's OK. Perhaps something got fixed in the gst-fsl-plugins bump.
> 
> Is this a Acked-by for the patch?

Ping ? :-)

Thomas
Thomas Petazzoni Oct. 6, 2014, 3:18 p.m. UTC | #5
Dear Peter Seiderer,

On Tue, 25 Feb 2014 23:18:42 +0100, Peter Seiderer wrote:
> The v4l2 ioctl VIDIOC_DBG_G_CHIP_IDENT was removed in linux-3.11 (see [1]/[2]).
> 
> To avoid build failure of the gst-fsl-plugins/mfw_gst_v4lsrc with toolchains based
> on newer linux kernel headers this patch gives the include of the custom linux
> kernel headers precedence over the sysroot linux kernel headers (oposed to the
> previous behavior).
> 
> Fixes the following compile error with buildroot/glibc toolchain:
> 
> mfw_gst_v4lsrc.c: In function 'mfw_gst_v4lsrc_capture_setup':
> mfw_gst_v4lsrc.c:452:10: error: variable 'chip' has initializer but incomplete type
>    struct v4l2_dbg_chip_ident chip = {0};
>           ^
> mfw_gst_v4lsrc.c:452:10: warning: excess elements in struct initializer [enabled by default]
> mfw_gst_v4lsrc.c:452:10: warning: (near initialization for 'chip') [enabled by default]
> mfw_gst_v4lsrc.c:452:30: error: storage size of 'chip' isn't known
>    struct v4l2_dbg_chip_ident chip = {0};
>                               ^
> mfw_gst_v4lsrc.c:464:22: error: 'VIDIOC_DBG_G_CHIP_IDENT' undeclared (first use in this function)
>    if (ioctl (fd_v4l, VIDIOC_DBG_G_CHIP_IDENT, &chip)) {
>                       ^
> 
> [1] http://www.spinics.net/lists/linux-media/msg64081.html
> [2] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/videodev2.h?id=b71c99801e18eb172ae34851daf25044a3bf644a
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> ---
>  package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I believe this problem has been fixed by the following commit:

commit daf3708be46d7eb7e9352ee987f37e32eb00c6bd
Author: Gary Bisson <bisson.gary@gmail.com>
Date:   Fri Jul 25 22:34:30 2014 -0700

    gst-fsl-plugins: fix build for kernel headers >= 3.10
    
    This package couldn't build using a toolchain with kernel headers >=
    3.10 because of the use of VIDIOC_DBG_G_CHIP_IDENT. As stated in
    videodev2.h, this ioctl should never be used.
    
    Signed-off-by: Gary Bisson <bisson.gary@gmail.com>
    Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
    Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Therefore, I'm marking your patch as Superseded in our patch tracking
system.

Best regards,

Thomas
diff mbox

Patch

diff --git a/package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk b/package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk
index ccdb521..610bd78 100644
--- a/package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk
+++ b/package/gstreamer/gst-fsl-plugins/gst-fsl-plugins.mk
@@ -23,7 +23,7 @@  GST_FSL_PLUGINS_CONF_ENV = PLATFORM=$(BR2_PACKAGE_GST_FSL_PLUGINS_PLATFORM)
 
 # needs access to imx-specific kernel headers
 GST_FSL_PLUGINS_DEPENDENCIES += linux
-GST_FSL_PLUGINS_CONF_ENV += CPPFLAGS="$(TARGET_CPPFLAGS) -idirafter $(LINUX_DIR)/include"
+GST_FSL_PLUGINS_CONF_ENV += CPPFLAGS="$(TARGET_CPPFLAGS) -I$(LINUX_DIR)/include"
 
 ifeq ($(BR2_PACKAGE_XLIB_LIBX11),y)
 GST_FSL_PLUGINS_DEPENDENCIES += xlib_libX11