Re: [RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
Hi,
On 08/09/2012 02:14 PM, Hans Verkuil wrote:
On Thu August 9 2012 13:58:07 Hans de Goede wrote:
Hi Konke,
As Gregor already mentioned there is no need to define libv4l2rdssubdir in
configure.ac ,
so please drop that.
Other then that I've some minor remarks (comments inline), with all those
fixed, this one is could to go. So hopefully the next version can be added
to git master!
On 08/07/2012 05:11 PM, Konke Radlow wrote:
---
Makefile.am |3 +-
configure.ac|7 +-
lib/include/libv4l2rds.h| 228 ++
lib/libv4l2rds/Makefile.am | 11 +
lib/libv4l2rds/libv4l2rds.c | 953
+++
lib/libv4l2rds/libv4l2rds.pc.in | 11 +
6 files changed, 1211 insertions(+), 2 deletions(-)
create mode 100644 lib/include/libv4l2rds.h
create mode 100644 lib/libv4l2rds/Makefile.am
create mode 100644 lib/libv4l2rds/libv4l2rds.c
create mode 100644 lib/libv4l2rds/libv4l2rds.pc.in
snip
diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h
new file mode 100644
index 000..4aa8593
--- /dev/null
+++ b/lib/include/libv4l2rds.h
@@ -0,0 +1,228 @@
+/*
+ * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights
reserved.
+ * Author: Konke Radlow korad...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA
+ */
+
+#ifndef __LIBV4L2RDS
+#define __LIBV4L2RDS
+
+#include errno.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include stdbool.h
+#include unistd.h
+#include stdint.h
+#include time.h
+#include sys/types.h
+#include sys/mman.h
+#include config.h
You should never include config.h in a public header, also
are all the headers really needed for the prototypes in this header?
I don't think so! Please move all the unneeded ones to the libv4l2rds.c
file!
+
+#include linux/videodev2.h
+
+#ifdef __cplusplus
+extern C {
+#endif /* __cplusplus */
+
+#if HAVE_VISIBILITY
+#define LIBV4L_PUBLIC __attribute__ ((visibility(default)))
+#else
+#define LIBV4L_PUBLIC
+#endif
+
+/* used to define the current version (version field) of the v4l2_rds struct */
+#define V4L2_RDS_VERSION (1)
+
What is the purpose of this field? Once we've released a v4l-utils with this
library we are stuck to the API we've defined, having a version field
changing it,
won't stop us from breaking existing apps, so once we've an official release we
simply cannot make ABI breaking changes, which is why most of my review sofar
has concentrated on the API side :)
I suggest dropping this define and the version field from the struct.
I think it is useful, actually. The v4l2_rds struct is allocated by the
v4l2_rds_create
so at least in theory it is possible to extend the struct in the future without
breaking
existing apps, provided you have a version number to check.
I disagree, if it gets extended only, then existing apps will just work, if an
apps gets
compiled against a newer version with the extension then it is safe to assume
it will run
against that newer version. The only reason I can see a version define being
useful is
to make a newer app compile with an older version of the librarry, but that
only requires
a version define, not a version field in the struct.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
On Fri 10 August 2012 09:16:34 Hans de Goede wrote:
Hi,
On 08/09/2012 02:14 PM, Hans Verkuil wrote:
On Thu August 9 2012 13:58:07 Hans de Goede wrote:
Hi Konke,
As Gregor already mentioned there is no need to define libv4l2rdssubdir in
configure.ac ,
so please drop that.
Other then that I've some minor remarks (comments inline), with all those
fixed, this one is could to go. So hopefully the next version can be added
to git master!
On 08/07/2012 05:11 PM, Konke Radlow wrote:
---
Makefile.am |3 +-
configure.ac|7 +-
lib/include/libv4l2rds.h| 228 ++
lib/libv4l2rds/Makefile.am | 11 +
lib/libv4l2rds/libv4l2rds.c | 953
+++
lib/libv4l2rds/libv4l2rds.pc.in | 11 +
6 files changed, 1211 insertions(+), 2 deletions(-)
create mode 100644 lib/include/libv4l2rds.h
create mode 100644 lib/libv4l2rds/Makefile.am
create mode 100644 lib/libv4l2rds/libv4l2rds.c
create mode 100644 lib/libv4l2rds/libv4l2rds.pc.in
snip
diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h
new file mode 100644
index 000..4aa8593
--- /dev/null
+++ b/lib/include/libv4l2rds.h
@@ -0,0 +1,228 @@
+/*
+ * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights
reserved.
+ * Author: Konke Radlow korad...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as
published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA
02110-1335 USA
+ */
+
+#ifndef __LIBV4L2RDS
+#define __LIBV4L2RDS
+
+#include errno.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include stdbool.h
+#include unistd.h
+#include stdint.h
+#include time.h
+#include sys/types.h
+#include sys/mman.h
+#include config.h
You should never include config.h in a public header, also
are all the headers really needed for the prototypes in this header?
I don't think so! Please move all the unneeded ones to the libv4l2rds.c
file!
+
+#include linux/videodev2.h
+
+#ifdef __cplusplus
+extern C {
+#endif /* __cplusplus */
+
+#if HAVE_VISIBILITY
+#define LIBV4L_PUBLIC __attribute__ ((visibility(default)))
+#else
+#define LIBV4L_PUBLIC
+#endif
+
+/* used to define the current version (version field) of the v4l2_rds
struct */
+#define V4L2_RDS_VERSION (1)
+
What is the purpose of this field? Once we've released a v4l-utils with
this
library we are stuck to the API we've defined, having a version field
changing it,
won't stop us from breaking existing apps, so once we've an official
release we
simply cannot make ABI breaking changes, which is why most of my review
sofar
has concentrated on the API side :)
I suggest dropping this define and the version field from the struct.
I think it is useful, actually. The v4l2_rds struct is allocated by the
v4l2_rds_create
so at least in theory it is possible to extend the struct in the future
without breaking
existing apps, provided you have a version number to check.
I disagree, if it gets extended only, then existing apps will just work, if
an apps gets
compiled against a newer version with the extension then it is safe to assume
it will run
against that newer version. The only reason I can see a version define being
useful is
to make a newer app compile with an older version of the librarry, but that
only requires
a version define, not a version field in the struct.
That's true, you only need the define, not the version field.
So let's keep the define and ditch the version field. I think that
should do it.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
Hi,
On 08/10/2012 09:36 AM, Hans Verkuil wrote:
On Fri 10 August 2012 09:16:34 Hans de Goede wrote:
Hi,
On 08/09/2012 02:14 PM, Hans Verkuil wrote:
On Thu August 9 2012 13:58:07 Hans de Goede wrote:
Hi Konke,
As Gregor already mentioned there is no need to define libv4l2rdssubdir in
configure.ac ,
so please drop that.
Other then that I've some minor remarks (comments inline), with all those
fixed, this one is could to go. So hopefully the next version can be added
to git master!
On 08/07/2012 05:11 PM, Konke Radlow wrote:
---
Makefile.am |3 +-
configure.ac|7 +-
lib/include/libv4l2rds.h| 228 ++
lib/libv4l2rds/Makefile.am | 11 +
lib/libv4l2rds/libv4l2rds.c | 953
+++
lib/libv4l2rds/libv4l2rds.pc.in | 11 +
6 files changed, 1211 insertions(+), 2 deletions(-)
create mode 100644 lib/include/libv4l2rds.h
create mode 100644 lib/libv4l2rds/Makefile.am
create mode 100644 lib/libv4l2rds/libv4l2rds.c
create mode 100644 lib/libv4l2rds/libv4l2rds.pc.in
snip
diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h
new file mode 100644
index 000..4aa8593
--- /dev/null
+++ b/lib/include/libv4l2rds.h
@@ -0,0 +1,228 @@
+/*
+ * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights
reserved.
+ * Author: Konke Radlow korad...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA
+ */
+
+#ifndef __LIBV4L2RDS
+#define __LIBV4L2RDS
+
+#include errno.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include stdbool.h
+#include unistd.h
+#include stdint.h
+#include time.h
+#include sys/types.h
+#include sys/mman.h
+#include config.h
You should never include config.h in a public header, also
are all the headers really needed for the prototypes in this header?
I don't think so! Please move all the unneeded ones to the libv4l2rds.c
file!
+
+#include linux/videodev2.h
+
+#ifdef __cplusplus
+extern C {
+#endif /* __cplusplus */
+
+#if HAVE_VISIBILITY
+#define LIBV4L_PUBLIC __attribute__ ((visibility(default)))
+#else
+#define LIBV4L_PUBLIC
+#endif
+
+/* used to define the current version (version field) of the v4l2_rds struct */
+#define V4L2_RDS_VERSION (1)
+
What is the purpose of this field? Once we've released a v4l-utils with this
library we are stuck to the API we've defined, having a version field
changing it,
won't stop us from breaking existing apps, so once we've an official release we
simply cannot make ABI breaking changes, which is why most of my review sofar
has concentrated on the API side :)
I suggest dropping this define and the version field from the struct.
I think it is useful, actually. The v4l2_rds struct is allocated by the
v4l2_rds_create
so at least in theory it is possible to extend the struct in the future without
breaking
existing apps, provided you have a version number to check.
I disagree, if it gets extended only, then existing apps will just work, if an
apps gets
compiled against a newer version with the extension then it is safe to assume
it will run
against that newer version. The only reason I can see a version define being
useful is
to make a newer app compile with an older version of the librarry, but that
only requires
a version define, not a version field in the struct.
That's true, you only need the define, not the version field.
So let's keep the define and ditch the version field. I think that
should do it.
Ack.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
Hello Konke, On 8/7/12 5:11 PM, Konke Radlow wrote: diff --git a/configure.ac b/configure.ac index 8ddcc9d..1109c4d 100644 --- a/configure.ac +++ b/configure.ac @@ -146,9 +148,12 @@ AC_ARG_WITH(libv4l2subdir, AS_HELP_STRING(--with-libv4l2subdir=DIR,set libv4l2 l AC_ARG_WITH(libv4lconvertsubdir, AS_HELP_STRING(--with-libv4lconvertsubdir=DIR,set libv4lconvert library subdir [default=libv4l]), libv4lconvertsubdir=$withval, libv4lconvertsubdir=libv4l) +AC_ARG_WITH(libv4l2rdssubdir, AS_HELP_STRING(--with-libv4l2rdssubdir=DIR,set libv4l2rds library subdir [default=libv4l]), + libv4l2rdssubdir=$withval, libv4l2rdssubdir=libv4l) + AC_ARG_WITH(udevdir, AS_HELP_STRING(--with-udevdir=DIR,set udev directory [default=/lib/udev]), udevdir=$withval, udevdir=/lib/udev) - + libv4l1privdir=$libdir/$libv4l1subdir libv4l2privdir=$libdir/$libv4l2subdir libv4l2plugindir=$libv4l2privdir/plugins please remove these changes. They are not needed for the RDS library. Thanks, Gregor -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
Hi Konke,
As Gregor already mentioned there is no need to define libv4l2rdssubdir in
configure.ac ,
so please drop that.
Other then that I've some minor remarks (comments inline), with all those
fixed, this one is could to go. So hopefully the next version can be added
to git master!
On 08/07/2012 05:11 PM, Konke Radlow wrote:
---
Makefile.am |3 +-
configure.ac|7 +-
lib/include/libv4l2rds.h| 228 ++
lib/libv4l2rds/Makefile.am | 11 +
lib/libv4l2rds/libv4l2rds.c | 953 +++
lib/libv4l2rds/libv4l2rds.pc.in | 11 +
6 files changed, 1211 insertions(+), 2 deletions(-)
create mode 100644 lib/include/libv4l2rds.h
create mode 100644 lib/libv4l2rds/Makefile.am
create mode 100644 lib/libv4l2rds/libv4l2rds.c
create mode 100644 lib/libv4l2rds/libv4l2rds.pc.in
snip
diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h
new file mode 100644
index 000..4aa8593
--- /dev/null
+++ b/lib/include/libv4l2rds.h
@@ -0,0 +1,228 @@
+/*
+ * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights
reserved.
+ * Author: Konke Radlow korad...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA
+ */
+
+#ifndef __LIBV4L2RDS
+#define __LIBV4L2RDS
+
+#include errno.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include stdbool.h
+#include unistd.h
+#include stdint.h
+#include time.h
+#include sys/types.h
+#include sys/mman.h
+#include config.h
You should never include config.h in a public header, also
are all the headers really needed for the prototypes in this header?
I don't think so! Please move all the unneeded ones to the libv4l2rds.c
file!
+
+#include linux/videodev2.h
+
+#ifdef __cplusplus
+extern C {
+#endif /* __cplusplus */
+
+#if HAVE_VISIBILITY
+#define LIBV4L_PUBLIC __attribute__ ((visibility(default)))
+#else
+#define LIBV4L_PUBLIC
+#endif
+
+/* used to define the current version (version field) of the v4l2_rds struct */
+#define V4L2_RDS_VERSION (1)
+
What is the purpose of this field? Once we've released a v4l-utils with this
library we are stuck to the API we've defined, having a version field
changing it,
won't stop us from breaking existing apps, so once we've an official release we
simply cannot make ABI breaking changes, which is why most of my review sofar
has concentrated on the API side :)
I suggest dropping this define and the version field from the struct.
+/* Constants used to define the size of arrays used to store RDS information */
+#define MAX_ODA_CNT 18 /* there are 16 groups each with type a or b.
Of these
+* 32 distinct groups, 18 can be used for ODA purposes
*/
+#define MAX_AF_CNT 25 /* AF Method A allows a maximum of 25 AFs to be defined
+* AF Method B does not impose a limit on the number of
AFs
+* but it is not fully supported at the moment and will
+* not receive more than 25 AFs */
+
+/* Define Constants for the possible types of RDS information
+ * used to address the relevant bit in the valid_fields bitmask */
+#define V4L2_RDS_PI0x01/* Program Identification */
+#define V4L2_RDS_PTY 0x02/* Program Type */
+#define V4L2_RDS_TP0x04/* Traffic Program */
+#define V4L2_RDS_PS0x08/* Program Service Name */
+#define V4L2_RDS_TA0x10/* Traffic Announcement */
+#define V4L2_RDS_DI0x20/* Decoder Information */
+#define V4L2_RDS_MS0x40/* Music / Speech flag */
+#define V4L2_RDS_PTYN 0x80/* Program Type Name */
+#define V4L2_RDS_RT0x100 /* Radio-Text */
+#define V4L2_RDS_TIME 0x200 /* Date and Time information */
+#define V4L2_RDS_TMC 0x400 /* TMC availability */
+#define V4L2_RDS_AF0x800 /* AF (alternative freq) available */
+#define V4L2_RDS_ECC 0x1000 /* Extended County Code */
+#define V4L2_RDS_LC0x2000 /* Language Code */
+
+/* Define Constants for the state of the RDS decoding process
+ * used to address the relevant bit in the decode_information bitmask */
+#define V4L2_RDS_GROUP_NEW 0x01/* New group received */
Re: [RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
On Thu August 9 2012 13:58:07 Hans de Goede wrote:
Hi Konke,
As Gregor already mentioned there is no need to define libv4l2rdssubdir in
configure.ac ,
so please drop that.
Other then that I've some minor remarks (comments inline), with all those
fixed, this one is could to go. So hopefully the next version can be added
to git master!
On 08/07/2012 05:11 PM, Konke Radlow wrote:
---
Makefile.am |3 +-
configure.ac|7 +-
lib/include/libv4l2rds.h| 228 ++
lib/libv4l2rds/Makefile.am | 11 +
lib/libv4l2rds/libv4l2rds.c | 953
+++
lib/libv4l2rds/libv4l2rds.pc.in | 11 +
6 files changed, 1211 insertions(+), 2 deletions(-)
create mode 100644 lib/include/libv4l2rds.h
create mode 100644 lib/libv4l2rds/Makefile.am
create mode 100644 lib/libv4l2rds/libv4l2rds.c
create mode 100644 lib/libv4l2rds/libv4l2rds.pc.in
snip
diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h
new file mode 100644
index 000..4aa8593
--- /dev/null
+++ b/lib/include/libv4l2rds.h
@@ -0,0 +1,228 @@
+/*
+ * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights
reserved.
+ * Author: Konke Radlow korad...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as
published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335
USA
+ */
+
+#ifndef __LIBV4L2RDS
+#define __LIBV4L2RDS
+
+#include errno.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include stdbool.h
+#include unistd.h
+#include stdint.h
+#include time.h
+#include sys/types.h
+#include sys/mman.h
+#include config.h
You should never include config.h in a public header, also
are all the headers really needed for the prototypes in this header?
I don't think so! Please move all the unneeded ones to the libv4l2rds.c
file!
+
+#include linux/videodev2.h
+
+#ifdef __cplusplus
+extern C {
+#endif /* __cplusplus */
+
+#if HAVE_VISIBILITY
+#define LIBV4L_PUBLIC __attribute__ ((visibility(default)))
+#else
+#define LIBV4L_PUBLIC
+#endif
+
+/* used to define the current version (version field) of the v4l2_rds
struct */
+#define V4L2_RDS_VERSION (1)
+
What is the purpose of this field? Once we've released a v4l-utils with this
library we are stuck to the API we've defined, having a version field
changing it,
won't stop us from breaking existing apps, so once we've an official release
we
simply cannot make ABI breaking changes, which is why most of my review sofar
has concentrated on the API side :)
I suggest dropping this define and the version field from the struct.
I think it is useful, actually. The v4l2_rds struct is allocated by the
v4l2_rds_create
so at least in theory it is possible to extend the struct in the future without
breaking
existing apps, provided you have a version number to check.
Regards,
Hans
+/* Constants used to define the size of arrays used to store RDS
information */
+#define MAX_ODA_CNT 18 /* there are 16 groups each with type a or b.
Of these
+* 32 distinct groups, 18 can be used for ODA purposes
*/
+#define MAX_AF_CNT 25 /* AF Method A allows a maximum of 25 AFs to be
defined
+* AF Method B does not impose a limit on the number of
AFs
+* but it is not fully supported at the moment and will
+* not receive more than 25 AFs */
+
+/* Define Constants for the possible types of RDS information
+ * used to address the relevant bit in the valid_fields bitmask */
+#define V4L2_RDS_PI0x01/* Program Identification */
+#define V4L2_RDS_PTY 0x02/* Program Type */
+#define V4L2_RDS_TP0x04/* Traffic Program */
+#define V4L2_RDS_PS0x08/* Program Service Name */
+#define V4L2_RDS_TA0x10/* Traffic Announcement */
+#define V4L2_RDS_DI0x20/* Decoder Information */
+#define V4L2_RDS_MS0x40/* Music / Speech flag */
+#define V4L2_RDS_PTYN 0x80/* Program Type Name */
+#define V4L2_RDS_RT0x100 /*
Re: [RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
On Tue August 7 2012 17:11:54 Konke Radlow wrote:
---
Makefile.am |3 +-
configure.ac|7 +-
lib/include/libv4l2rds.h| 228 ++
lib/libv4l2rds/Makefile.am | 11 +
lib/libv4l2rds/libv4l2rds.c | 953
+++
lib/libv4l2rds/libv4l2rds.pc.in | 11 +
6 files changed, 1211 insertions(+), 2 deletions(-)
create mode 100644 lib/include/libv4l2rds.h
create mode 100644 lib/libv4l2rds/Makefile.am
create mode 100644 lib/libv4l2rds/libv4l2rds.c
create mode 100644 lib/libv4l2rds/libv4l2rds.pc.in
diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h
new file mode 100644
index 000..4aa8593
--- /dev/null
+++ b/lib/include/libv4l2rds.h
@@ -0,0 +1,228 @@
+/*
+ * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights
reserved.
+ * Author: Konke Radlow korad...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335
USA
+ */
+
+#ifndef __LIBV4L2RDS
+#define __LIBV4L2RDS
+
+#include errno.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include stdbool.h
+#include unistd.h
+#include stdint.h
+#include time.h
+#include sys/types.h
+#include sys/mman.h
+#include config.h
+
+#include linux/videodev2.h
+
+#ifdef __cplusplus
+extern C {
+#endif /* __cplusplus */
+
+#if HAVE_VISIBILITY
+#define LIBV4L_PUBLIC __attribute__ ((visibility(default)))
+#else
+#define LIBV4L_PUBLIC
+#endif
+
+/* used to define the current version (version field) of the v4l2_rds struct
*/
+#define V4L2_RDS_VERSION (1)
+
+/* Constants used to define the size of arrays used to store RDS information
*/
+#define MAX_ODA_CNT 18 /* there are 16 groups each with type a or b.
Of these
+ * 32 distinct groups, 18 can be used for ODA purposes
*/
+#define MAX_AF_CNT 25/* AF Method A allows a maximum of 25 AFs to be
defined
+ * AF Method B does not impose a limit on the number of
AFs
+ * but it is not fully supported at the moment and will
+ * not receive more than 25 AFs */
+
+/* Define Constants for the possible types of RDS information
+ * used to address the relevant bit in the valid_fields bitmask */
+#define V4L2_RDS_PI 0x01/* Program Identification */
+#define V4L2_RDS_PTY 0x02/* Program Type */
+#define V4L2_RDS_TP 0x04/* Traffic Program */
+#define V4L2_RDS_PS 0x08/* Program Service Name */
+#define V4L2_RDS_TA 0x10/* Traffic Announcement */
+#define V4L2_RDS_DI 0x20/* Decoder Information */
+#define V4L2_RDS_MS 0x40/* Music / Speech flag */
+#define V4L2_RDS_PTYN0x80/* Program Type Name */
+#define V4L2_RDS_RT 0x100 /* Radio-Text */
+#define V4L2_RDS_TIME0x200 /* Date and Time information */
+#define V4L2_RDS_TMC 0x400 /* TMC availability */
+#define V4L2_RDS_AF 0x800 /* AF (alternative freq) available */
+#define V4L2_RDS_ECC 0x1000 /* Extended County Code */
+#define V4L2_RDS_LC 0x2000 /* Language Code */
+
+/* Define Constants for the state of the RDS decoding process
+ * used to address the relevant bit in the decode_information bitmask */
+#define V4L2_RDS_GROUP_NEW 0x01/* New group received */
+#define V4L2_RDS_ODA 0x02/* Open Data Group announced */
+
+/* Decoder Information (DI) codes
+ * used to decode the DI information according to the RDS standard */
+#define V4L2_RDS_FLAG_STEREO 0x01
+#define V4L2_RDS_FLAG_ARTIFICIAL_HEAD0x02
+#define V4L2_RDS_FLAG_COMPRESSED 0x04
+#define V4L2_RDS_FLAG_STATIC_PTY 0x08
+
+/* struct to encapsulate one complete RDS group */
+/* This structure is used internally to store data until a complete RDS
+ * group was received and group id dependent decoding can be done.
+ * It is also used to provide external access to uninterpreted RDS groups
+ * when manual decoding is required (e.g. special ODA types) */
+struct v4l2_rds_group {
+ uint16_t pi;/* Program Identification */
+ char group_version; /* group version ('A' /
Re: [RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
they are gone. And btw: I'm sorry for fiddling with your build environment in such a way. I have to admit that the additions I made were based more on copying from existing libraries than really understanding the effects of each command. Regards, Konke On Thu, Aug 9, 2012 at 7:22 AM, Gregor Jasny gja...@googlemail.com wrote: Hello Konke, On 8/7/12 5:11 PM, Konke Radlow wrote: diff --git a/configure.ac b/configure.ac index 8ddcc9d..1109c4d 100644 --- a/configure.ac +++ b/configure.ac @@ -146,9 +148,12 @@ AC_ARG_WITH(libv4l2subdir, AS_HELP_STRING(--with-libv4l2subdir=DIR,set libv4l2 l AC_ARG_WITH(libv4lconvertsubdir, AS_HELP_STRING(--with-libv4lconvertsubdir=DIR,set libv4lconvert library subdir [default=libv4l]), libv4lconvertsubdir=$withval, libv4lconvertsubdir=libv4l) +AC_ARG_WITH(libv4l2rdssubdir, AS_HELP_STRING(--with-libv4l2rdssubdir=DIR,set libv4l2rds library subdir [default=libv4l]), + libv4l2rdssubdir=$withval, libv4l2rdssubdir=libv4l) + AC_ARG_WITH(udevdir, AS_HELP_STRING(--with-udevdir=DIR,set udev directory [default=/lib/udev]), udevdir=$withval, udevdir=/lib/udev) - + libv4l1privdir=$libdir/$libv4l1subdir libv4l2privdir=$libdir/$libv4l2subdir libv4l2plugindir=$libv4l2privdir/plugins please remove these changes. They are not needed for the RDS library. Thanks, Gregor -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
On Thu, Aug 9, 2012 at 11:58 AM, Hans de Goede hdego...@redhat.com wrote:
Other then that I've some minor remarks (comments inline), with all those
fixed, this one is could to go. So hopefully the next version can be added
to git master!
I'm very happy to hear that :)
On 08/07/2012 05:11 PM, Konke Radlow wrote:
---
Makefile.am |3 +-
configure.ac|7 +-
lib/include/libv4l2rds.h| 228 ++
lib/libv4l2rds/Makefile.am | 11 +
lib/libv4l2rds/libv4l2rds.c | 953
+++
lib/libv4l2rds/libv4l2rds.pc.in | 11 +
6 files changed, 1211 insertions(+), 2 deletions(-)
create mode 100644 lib/include/libv4l2rds.h
create mode 100644 lib/libv4l2rds/Makefile.am
create mode 100644 lib/libv4l2rds/libv4l2rds.c
create mode 100644 lib/libv4l2rds/libv4l2rds.pc.in
snip
diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h
new file mode 100644
index 000..4aa8593
--- /dev/null
+++ b/lib/include/libv4l2rds.h
@@ -0,0 +1,228 @@
+/*
+ * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights
reserved.
+ * Author: Konke Radlow korad...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as
published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA
02110-1335 USA
+ */
+
+#ifndef __LIBV4L2RDS
+#define __LIBV4L2RDS
+
+#include errno.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include stdbool.h
+#include unistd.h
+#include stdint.h
+#include time.h
+#include sys/types.h
+#include sys/mman.h
+#include config.h
You should never include config.h in a public header, also
are all the headers really needed for the prototypes in this header?
I don't think so! Please move all the unneeded ones to the libv4l2rds.c
file!
That's true, I don't know why I had all these include directives in the public
header. After cleaning up we're left with only three (stdbool, stdint
and videodev2)
+
+#include linux/videodev2.h
+
+#ifdef __cplusplus
+extern C {
+#endif /* __cplusplus */
+
+#if HAVE_VISIBILITY
+#define LIBV4L_PUBLIC __attribute__ ((visibility(default)))
+#else
+#define LIBV4L_PUBLIC
+#endif
+
+/* used to define the current version (version field) of the v4l2_rds
struct */
+#define V4L2_RDS_VERSION (1)
+
What is the purpose of this field? Once we've released a v4l-utils with this
library we are stuck to the API we've defined, having a version field
changing it,
won't stop us from breaking existing apps, so once we've an official release
we
simply cannot make ABI breaking changes, which is why most of my review
sofar
has concentrated on the API side :)
I suggest dropping this define and the version field from the struct.
As Hans mentioned this version field is not supposed to denote the API version,
but rather the version of the v4l2_rds_handle struct. This struct could then
theoretically be updated in the future and provide applications a way
of verifying
their compatibility with the rds-library version installed on the system.
+/* Constants used to define the size of arrays used to store RDS
information */
+#define MAX_ODA_CNT 18 /* there are 16 groups each with type a or
b. Of these
+* 32 distinct groups, 18 can be used for ODA
purposes */
+#define MAX_AF_CNT 25 /* AF Method A allows a maximum of 25 AFs to be
defined
+* AF Method B does not impose a limit on the
number of AFs
+* but it is not fully supported at the moment and
will
+* not receive more than 25 AFs */
+
+/* Define Constants for the possible types of RDS information
+ * used to address the relevant bit in the valid_fields bitmask */
+#define V4L2_RDS_PI0x01/* Program Identification */
+#define V4L2_RDS_PTY 0x02/* Program Type */
+#define V4L2_RDS_TP0x04/* Traffic Program */
+#define V4L2_RDS_PS0x08/* Program Service Name */
+#define V4L2_RDS_TA0x10/* Traffic Announcement */
+#define V4L2_RDS_DI0x20/* Decoder Information */
+#define V4L2_RDS_MS0x40/* Music / Speech flag */
+#define V4L2_RDS_PTYN 0x80/* Program Type Name */
+#define V4L2_RDS_RT
[RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
---
Makefile.am |3 +-
configure.ac|7 +-
lib/include/libv4l2rds.h| 228 ++
lib/libv4l2rds/Makefile.am | 11 +
lib/libv4l2rds/libv4l2rds.c | 953 +++
lib/libv4l2rds/libv4l2rds.pc.in | 11 +
6 files changed, 1211 insertions(+), 2 deletions(-)
create mode 100644 lib/include/libv4l2rds.h
create mode 100644 lib/libv4l2rds/Makefile.am
create mode 100644 lib/libv4l2rds/libv4l2rds.c
create mode 100644 lib/libv4l2rds/libv4l2rds.pc.in
diff --git a/Makefile.am b/Makefile.am
index a754955..621d8d9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5,7 +5,8 @@ SUBDIRS = \
lib/libv4lconvert \
lib/libv4l2 \
lib/libv4l1 \
- lib/libdvbv5
+ lib/libdvbv5 \
+ lib/libv4l2rds
if WITH_V4LUTILS
SUBDIRS += \
diff --git a/configure.ac b/configure.ac
index 8ddcc9d..1109c4d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -14,6 +14,7 @@ AC_CONFIG_FILES([Makefile
lib/libv4l2/Makefile
lib/libv4l1/Makefile
lib/libdvbv5/Makefile
+ lib/libv4l2rds/Makefile
utils/libv4l2util/Makefile
utils/libmedia_dev/Makefile
@@ -36,6 +37,7 @@ AC_CONFIG_FILES([Makefile
lib/libv4l1/libv4l1.pc
lib/libv4l2/libv4l2.pc
lib/libdvbv5/libdvbv5.pc
+ lib/libv4l2rds/libv4l2rds.pc
])
AM_INIT_AUTOMAKE([1.9 no-dist-gzip dist-bzip2 -Wno-portability]) # 1.10 is
needed for target_LIBTOOLFLAGS
@@ -146,9 +148,12 @@ AC_ARG_WITH(libv4l2subdir,
AS_HELP_STRING(--with-libv4l2subdir=DIR,set libv4l2 l
AC_ARG_WITH(libv4lconvertsubdir,
AS_HELP_STRING(--with-libv4lconvertsubdir=DIR,set libv4lconvert library subdir
[default=libv4l]),
libv4lconvertsubdir=$withval, libv4lconvertsubdir=libv4l)
+AC_ARG_WITH(libv4l2rdssubdir, AS_HELP_STRING(--with-libv4l2rdssubdir=DIR,set
libv4l2rds library subdir [default=libv4l]),
+ libv4l2rdssubdir=$withval, libv4l2rdssubdir=libv4l)
+
AC_ARG_WITH(udevdir, AS_HELP_STRING(--with-udevdir=DIR,set udev directory
[default=/lib/udev]),
udevdir=$withval, udevdir=/lib/udev)
-
+
libv4l1privdir=$libdir/$libv4l1subdir
libv4l2privdir=$libdir/$libv4l2subdir
libv4l2plugindir=$libv4l2privdir/plugins
diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h
new file mode 100644
index 000..4aa8593
--- /dev/null
+++ b/lib/include/libv4l2rds.h
@@ -0,0 +1,228 @@
+/*
+ * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights
reserved.
+ * Author: Konke Radlow korad...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA
+ */
+
+#ifndef __LIBV4L2RDS
+#define __LIBV4L2RDS
+
+#include errno.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include stdbool.h
+#include unistd.h
+#include stdint.h
+#include time.h
+#include sys/types.h
+#include sys/mman.h
+#include config.h
+
+#include linux/videodev2.h
+
+#ifdef __cplusplus
+extern C {
+#endif /* __cplusplus */
+
+#if HAVE_VISIBILITY
+#define LIBV4L_PUBLIC __attribute__ ((visibility(default)))
+#else
+#define LIBV4L_PUBLIC
+#endif
+
+/* used to define the current version (version field) of the v4l2_rds struct */
+#define V4L2_RDS_VERSION (1)
+
+/* Constants used to define the size of arrays used to store RDS information */
+#define MAX_ODA_CNT 18 /* there are 16 groups each with type a or b.
Of these
+* 32 distinct groups, 18 can be used for ODA purposes
*/
+#define MAX_AF_CNT 25 /* AF Method A allows a maximum of 25 AFs to be defined
+* AF Method B does not impose a limit on the number of
AFs
+* but it is not fully supported at the moment and will
+* not receive more than 25 AFs */
+
+/* Define Constants for the possible types of RDS information
+ * used to address the relevant bit in the valid_fields bitmask */
+#define V4L2_RDS_PI0x01/* Program Identification */
+#define V4L2_RDS_PTY 0x02/* Program Type */
+#define V4L2_RDS_TP0x04/* Traffic Program */
+#define V4L2_RDS_PS0x08/* Program Service Name */
+#define V4L2_RDS_TA0x10/* Traffic Announcement */
+#define V4L2_RDS_DI0x20/* Decoder Information */
+#define V4L2_RDS_MS0x40
