GNOME Bugzilla – Bug 721869
dvbsrc needs to conditionally build for older dvb headers
Last modified: 2014-05-26 11:30:44 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
Given that this was introduced by e34df02115574084c13cc88caa47198bcc0856fc, in october, maybe disabling dvbsrc is best, as I've not seen this reported before...
While that patch was from October, it was pushed recently. I think we should make it work with older kernel too.
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.
Created attachment 265850 [details] [review] check if the headers are good Doing conditional compilation would be nicer, but this unbreaks us for now.
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 :)
Updated and pushed.
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.
*** Bug 722083 has been marked as a duplicate of this bug. ***
Vincent, I've pushed a patch to master which might fix the issue. If not, can you post the build errors here ?
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
Created attachment 274086 [details] [review] fix a bit more errors with 5.4
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.
Created attachment 274110 [details] [review] add compatibily check for older api
Created attachment 274112 [details] [review] add compatibily check for older api
Err ... We depend on minor 5 already in configure.ac No need to check for anything already provided by minor 5 and lower.
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.
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".
Edward, should we just close this then ? Do we have someone with 5.5 headers to check ?
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
Created attachment 274335 [details] [review] map A to AC
Created attachment 274336 [details] [review] typo fix in test
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.
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.
Created attachment 274339 [details] [review] Map A to AC for <5.6, not <5.5
Created attachment 274341 [details] [review] 5.6 also for comments... :P
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