After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 721869 - dvbsrc needs to conditionally build for older dvb headers
dvbsrc needs to conditionally build for older dvb headers
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.3.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 722083 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-01-09 15:19 UTC by Vincent Penquerc'h
Modified: 2014-05-26 11:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
check if the headers are good (1.19 KB, patch)
2014-01-09 15:40 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
committed Details | Review
fix a bit more errors with 5.4 (2.45 KB, patch)
2014-04-11 12:18 UTC, Vincent Penquerc'h
needs-work Details | Review
add compatibily check for older api (2.78 KB, patch)
2014-04-11 14:33 UTC, Stefan Ringel
none Details | Review
add compatibily check for older api (3.23 KB, patch)
2014-04-11 14:40 UTC, Stefan Ringel
none Details | Review
map A to AC (1.37 KB, patch)
2014-04-15 08:32 UTC, Vincent Penquerc'h
none Details | Review
typo fix in test (1022 bytes, patch)
2014-04-15 08:33 UTC, Vincent Penquerc'h
committed Details | Review
Map A to AC for <5.6, not <5.5 (1.37 KB, patch)
2014-04-15 09:04 UTC, Vincent Penquerc'h
none Details | Review
5.6 also for comments... :P (1.37 KB, patch)
2014-04-15 09:11 UTC, Vincent Penquerc'h
committed Details | Review

Description Vincent Penquerc'h 2014-01-09 15:19:09 UTC
Latest git fails in sys/dvbsrc, see errors at the end.
These SYS_* defines seem to be from /usr/include/linux/dvb/frontend.h, but I do not have all of them. My version's 3.2.0-58.88 from Ubuntu, presumably before the addition of these. I'm unsure if it'd be best to check for those missing ones in configure, or just disable dvbsrc for such old versions. I don't have a dvb hardware to test any changes.


  CC     libgstdvb_la-gstdvbsrc.lo
gstdvbsrc.c: In function 'gst_dvbsrc_delsys_get_type':
gstdvbsrc.c:286:6: error: 'SYS_DVBC_ANNEX_A' undeclared (first use in this function)
gstdvbsrc.c:286:6: note: each undeclared identifier is reported only once for each function it appears in
gstdvbsrc.c:298:6: error: 'SYS_DTMB' undeclared (first use in this function)
gstdvbsrc.c:303:6: error: 'SYS_DVBC_ANNEX_C' undeclared (first use in this function)
gstdvbsrc.c: In function 'gst_dvbsrc_class_init':
gstdvbsrc.c:558:22: error: 'NO_STREAM_ID_FILTER' undeclared (first use in this function)
gstdvbsrc.c:558:11: error: passing argument 6 of 'g_param_spec_int' makes integer from pointer without a cast [-Werror]
/usr/include/glib-2.0/gobject/gparamspecs.h:999:13: note: expected 'gint' but argument is of type 'struct GEnumValue *'
gstdvbsrc.c: In function 'gst_dvbsrc_init':
gstdvbsrc.c:607:23: error: 'NO_STREAM_ID_FILTER' undeclared (first use in this function)
gstdvbsrc.c:607:21: error: assignment makes integer from pointer without a cast [-Werror]
gstdvbsrc.c: In function 'gst_dvbsrc_open_frontend':
gstdvbsrc.c:930:21: error: 'DTV_ENUM_DELSYS' undeclared (first use in this function)
gstdvbsrc.c:930:19: error: assignment makes integer from pointer without a cast [-Werror]
gstdvbsrc.c:960:46: error: 'SYS_DVBC_ANNEX_A' undeclared (first use in this function)
gstdvbsrc.c:960:3: error: passing argument 2 of 'gst_dvbsrc_check_delsys' makes integer from pointer without a cast [-Werror]
gstdvbsrc.c:870:1: note: expected 'guchar' but argument is of type 'struct GEnumValue *'
gstdvbsrc.c:1006:46: error: 'SYS_DTMB' undeclared (first use in this function)
gstdvbsrc.c:1006:3: error: passing argument 2 of 'gst_dvbsrc_check_delsys' makes integer from pointer without a cast [-Werror]
gstdvbsrc.c:870:1: note: expected 'guchar' but argument is of type 'struct GEnumValue *'
gstdvbsrc.c:1023:46: error: 'SYS_DVBC_ANNEX_C' undeclared (first use in this function)
gstdvbsrc.c:1023:3: error: passing argument 2 of 'gst_dvbsrc_check_delsys' makes integer from pointer without a cast [-Werror]
gstdvbsrc.c:870:1: note: expected 'guchar' but argument is of type 'struct GEnumValue *'
gstdvbsrc.c: In function 'gst_dvbsrc_tune':
gstdvbsrc.c:1502:21: error: 'DTV_ENUM_DELSYS' undeclared (first use in this function)
gstdvbsrc.c:1502:19: error: assignment makes integer from pointer without a cast [-Werror]
gstdvbsrc.c:1575:35: error: 'DTV_STREAM_ID' undeclared (first use in this function)
gstdvbsrc.c:1575:11: error: passing argument 3 of 'set_prop' makes integer from pointer without a cast [-Werror]
gstdvbsrc.c:1457:1: note: expected 'guint32' but argument is of type 'struct GEnumValue *'
gstdvbsrc.c:1610:11: error: passing argument 3 of 'set_prop' makes integer from pointer without a cast [-Werror]
gstdvbsrc.c:1457:1: note: expected 'guint32' but argument is of type 'struct GEnumValue *'
gstdvbsrc.c:1615:12: error: 'SYS_DVBC_ANNEX_A' undeclared (first use in this function)
gstdvbsrc.c:1617:12: error: 'SYS_DVBC_ANNEX_C' undeclared (first use in this function)
cc1: all warnings being treated as errors
make: *** [libgstdvb_la-gstdvbsrc.lo] Error 1
Comment 1 Vincent Penquerc'h 2014-01-09 15:24:19 UTC
Given that this was introduced by e34df02115574084c13cc88caa47198bcc0856fc, in october, maybe disabling dvbsrc is best, as I've not seen this reported before...
Comment 2 Sebastian Dröge (slomo) 2014-01-09 15:25:30 UTC
While that patch was from October, it was pushed recently. I think we should make it work with older kernel too.
Comment 3 Vincent Penquerc'h 2014-01-09 15:30:42 UTC
OK. Given the type of the failure, that means the "delsys" type will have different allowed values depending on the version of the dvb headers used at build time, which might be a bit problematic.
I'll cook up a patch to do that in the meantime.
Comment 4 Stefan Sauer (gstreamer, gtkdoc dev) 2014-01-09 15:40:28 UTC
Created attachment 265850 [details] [review]
check if the headers are good

Doing conditional compilation would be nicer, but this unbreaks us for now.
Comment 5 Sebastian Dröge (slomo) 2014-01-09 15:50:14 UTC
Review of attachment 265850 [details] [review]:

::: configure.ac
@@ +2066,3 @@
   AC_MSG_CHECKING([Checking for up to date dvb installation])
+  AC_CHECK_HEADER(linux/dvb/frontend.h, [
+    AC_MSG_CHECKING([for uptodate dvb API])

up-to-date maybe? ;)

@@ +2068,3 @@
+    AC_MSG_CHECKING([for uptodate dvb API])
+    AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <linux/dvb/frontend.h>]], [[
+        #ifndef SYS_DVBC_ANNEX_A

This does not work because it is an enum value, not a #define. SYS_DVBC_ANNEX_AC is a #define though, or alternatively just try "int x = SYS_DVBC_ANNEX_A;"

Then push :)
Comment 6 Stefan Sauer (gstreamer, gtkdoc dev) 2014-01-09 15:59:59 UTC
Updated and pushed.
Comment 7 Stefan Ringel 2014-01-09 16:43:39 UTC
we must use FE_GET_PROPERTY cmd=DTV_API_VERSION to ask the frontend version.

if version are 5.10 then it will work

for older version use:

SYS_DVBC_ANNEX_AC not SYS_DVBC_ANNEX_A ,
SYS_DMBTH not SYS_DTMB ,

and not NO_STREAM_ID_FILTER

I hope, I have all.
Comment 8 Sebastian Dröge (slomo) 2014-01-13 08:33:57 UTC
*** Bug 722083 has been marked as a duplicate of this bug. ***
Comment 9 Edward Hervey 2014-04-11 08:19:42 UTC
Vincent, I've pushed a patch to master which might fix the issue.

If not, can you post the build errors here ?
Comment 10 Vincent Penquerc'h 2014-04-11 12:17:32 UTC
With your patch, configure doesn't select dvb to build, because I have version 5.4. If I build in sys/dvb anyway, I get the errors below.
As Stefan mentioned above, SYS_DVBC_ANNEX_AC is defined, fixed by the attached patch (though it's not clear whether it's any good, as it changes the API), but it's not clear what to do for _DELSYS, see after this log. _DELSYS isn't defined in 5.4, but apparently becomes available at 5.5.


  CC     libgstdvb_la-gstdvbsrc.lo
gstdvbsrc.c: In function 'gst_dvbsrc_delsys_get_type':
gstdvbsrc.c:361:6: error: 'SYS_DVBC_ANNEX_A' undeclared (first use in this function)
gstdvbsrc.c:361:6: note: each undeclared identifier is reported only once for each function it appears in
gstdvbsrc.c:380:6: error: 'SYS_DVBC_ANNEX_C' undeclared (first use in this function)
gstdvbsrc.c: In function 'gst_dvbsrc_open_frontend':
gstdvbsrc.c:1044:21: error: 'DTV_ENUM_DELSYS' undeclared (first use in this function)
gstdvbsrc.c:1044:19: error: assignment makes integer from pointer without a cast [-Werror]
gstdvbsrc.c:1074:46: error: 'SYS_DVBC_ANNEX_A' undeclared (first use in this function)
gstdvbsrc.c:1074:3: error: passing argument 2 of 'gst_dvbsrc_check_delsys' makes integer from pointer without a cast [-Werror]
gstdvbsrc.c:984:1: note: expected 'guchar' but argument is of type 'struct GEnumValue *'
gstdvbsrc.c:1139:46: error: 'SYS_DVBC_ANNEX_C' undeclared (first use in this function)
gstdvbsrc.c:1139:3: error: passing argument 2 of 'gst_dvbsrc_check_delsys' makes integer from pointer without a cast [-Werror]
gstdvbsrc.c:984:1: note: expected 'guchar' but argument is of type 'struct GEnumValue *'
gstdvbsrc.c: In function 'gst_dvbsrc_tune':
gstdvbsrc.c:1617:21: error: 'DTV_ENUM_DELSYS' undeclared (first use in this function)
gstdvbsrc.c:1617:19: error: assignment makes integer from pointer without a cast [-Werror]
gstdvbsrc.c:1709:12: error: 'SYS_DVBC_ANNEX_A' undeclared (first use in this function)
gstdvbsrc.c:1711:12: error: 'SYS_DVBC_ANNEX_C' undeclared (first use in this function)
cc1: all warnings being treated as errors
make: *** [libgstdvb_la-gstdvbsrc.lo] Error 1


With the patch I'm attaching in 10 seconds:

  CC     libgstdvb_la-gstdvbsrc.lo
gstdvbsrc.c: In function 'gst_dvbsrc_open_frontend':
gstdvbsrc.c:1050:21: error: 'DTV_ENUM_DELSYS' undeclared (first use in this function)
gstdvbsrc.c:1050:21: note: each undeclared identifier is reported only once for each function it appears in
gstdvbsrc.c: In function 'gst_dvbsrc_tune':
gstdvbsrc.c:1631:21: error: 'DTV_ENUM_DELSYS' undeclared (first use in this function)
make: *** [libgstdvb_la-gstdvbsrc.lo] Error 1
Comment 11 Vincent Penquerc'h 2014-04-11 12:18:36 UTC
Created attachment 274086 [details] [review]
fix a bit more errors with 5.4
Comment 12 Edward Hervey 2014-04-11 12:31:38 UTC
Review of attachment 274086 [details] [review]:

::: sys/dvb/gstdvbsrc.c
@@ +362,3 @@
     {SYS_DVBC_ANNEX_A, "DVB-C-A", "dvb-c-a"},
+#else
+    {SYS_DVBC_ANNEX_AC, "DVB-C-AC", "dvb-c-ac"},

I think it would be cleaner if we just defined SYS_DVBC_ANNEX_A (same value as the legacy SYS_DVBC_ANNEX_AC) instead of having one or the other.

It might just end up being way too confusing

This applies to all other usages of A/AC

@@ +385,2 @@
     {SYS_DVBC_ANNEX_C, "DVB-C-C", "dvb-c-c"},
+#endif

the DVB-C addition/protection this way is fine though.
Comment 13 Stefan Ringel 2014-04-11 14:33:02 UTC
Created attachment 274110 [details] [review]
add compatibily check for older api
Comment 14 Stefan Ringel 2014-04-11 14:40:34 UTC
Created attachment 274112 [details] [review]
add compatibily check for older api
Comment 15 Edward Hervey 2014-04-11 14:49:17 UTC
Err ... We depend on minor 5 already in configure.ac No need to check for anything already provided by minor 5 and lower.
Comment 16 Vincent Penquerc'h 2014-04-11 14:52:20 UTC
Almost! This second patch gives me:

  CC     libgstdvb_la-gstdvbsrc.lo
gstdvbsrc.c: In function 'gst_dvbsrc_open_frontend':
gstdvbsrc.c:1100:5: error: case value '5' not in enumerated type 'fe_type_t' [-Werror=switch]
gstdvbsrc.c:1110:5: error: case value '11' not in enumerated type 'fe_type_t' [-Werror=switch]
gstdvbsrc.c:1009:25: error: unused variable 'props' [-Werror=unused-variable]
cc1: all warnings being treated as errors
make: *** [libgstdvb_la-gstdvbsrc.lo] Error 1


SYS_DVBS and SYS_ATSC are in fe_delivery_system_t, not fe_type_t. The latter contains:
        FE_QPSK,
        FE_QAM,
        FE_OFDM,
        FE_ATSC    

dvb_frontend_info does not contain a fe_delivery_system_t field in my version.

I guess the other SYS_* are also wrong, but the enum values collide with existing fe_type_t ones and GCC ignores them.
Comment 17 Vincent Penquerc'h 2014-04-11 14:53:25 UTC
Yes, I have 5.4 here. If it's deemed too old, it's fine. FWIW, I'm the one who added the 5.5 check, which I chose as "I know my version is older than the current code, so I'll use the next one for now".
Comment 18 Vincent Penquerc'h 2014-04-14 16:20:32 UTC
Edward, should we just close this then ? Do we have someone with 5.5 headers to check ?
Comment 19 Edward Hervey 2014-04-15 05:54:50 UTC
The SYS_DVBC_ANNEX_A / _C was added in minor 6 from my findings.

But I would propose just fixing that by adding a simple
#ifndef SYS_DVBC_ANNEX_A
#define SYS_DVBC_ANNEX_AC SYS_DVB_ANNEX_A
#endif

And not expose the _C variant
Comment 20 Vincent Penquerc'h 2014-04-15 08:32:51 UTC
Created attachment 274335 [details] [review]
map A to AC
Comment 21 Vincent Penquerc'h 2014-04-15 08:33:28 UTC
Created attachment 274336 [details] [review]
typo fix in test
Comment 22 Vincent Penquerc'h 2014-04-15 08:35:03 UTC
Changed as requested. Comment 16 still applies, it's not clear what was intended. Also a small fix I spotted while reading the code. Once it builds here I'll also change configure to accept 5.4.
Comment 23 Edward Hervey 2014-04-15 09:00:46 UTC
Thanks for the updates !

The problem with going below 5.5 is the new delivery system, and I'm worried making that conditional would complexify the code too much.
Comment 24 Vincent Penquerc'h 2014-04-15 09:04:52 UTC
Created attachment 274339 [details] [review]
Map A to AC for <5.6, not <5.5
Comment 25 Vincent Penquerc'h 2014-04-15 09:11:20 UTC
Created attachment 274341 [details] [review]
5.6 also for comments... :P
Comment 26 Edward Hervey 2014-05-26 11:30:34 UTC
commit b67f64cf5dde04f23ec3b00e9be97eac12e58639
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Fri Apr 11 15:38:16 2014 +0100

    dvbsrc: map SYS_DVBC_ANNEX_A SYS_DVBC_ANNEX_AC for DVB API < 5.6
    
    https://bugzilla.gnome.org/show_bug.cgi?id=721869

commit 038d9794a1fb84e754de9bd07b1d8afad769218e
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Tue Apr 15 09:31:02 2014 +0100

    dvbsrc: fix typo in testing flag
    
    % instead of &
    
    https://bugzilla.gnome.org/show_bug.cgi?id=721869