[vlc-devel] [PATCH 1/3] Initial support for EIA-608 closed captions via V4L2 VBI devices

Rémi Denis-Courmont remi at remlab.net
Mon Oct 22 18:46:29 CEST 2012


Le dimanche 21 octobre 2012 23:01:00, Devin Heitmueller a écrit :
> This patch provides initial support for sending EIA-608 closed captions
> provided via the /dev/vbi interface to the VLC CC decoder.  It makes use of
> the ZVBI library, which is what does the VBI slicing.
> 
> Validated with the Hauppauge HVR-950q.
> ---
>  configure.ac                |    4 +-
>  modules/access/Modules.am   |    3 +-
>  modules/access/v4l2/demux.c |  127
> +++++++++++++++++++++++++++++++++++++++---- modules/access/v4l2/v4l2.c  | 
>  46 ++++++++++++++++
>  modules/access/v4l2/v4l2.h  |   14 +++++
>  modules/access/v4l2/vbi.c   |   86 +++++++++++++++++++++++++++++
>  6 files changed, 267 insertions(+), 13 deletions(-)
>  create mode 100644 modules/access/v4l2/vbi.c
> 
> diff --git a/configure.ac b/configure.ac
> index a748266..6ef4b11 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2800,11 +2800,11 @@ AC_ARG_ENABLE(telx,
>  AS_IF( [test "${enable_zvbi}" != "no"],[
>      PKG_CHECK_MODULES(ZVBI, [zvbi-0.2 >= 0.2.28],
>          [
> -          VLC_ADD_LIBS([zvbi],[$ZVBI_LIBS])
> +          VLC_ADD_LIBS([zvbi v4l2],[$ZVBI_LIBS])
>            if test "${SYS}" = "mingw32"; then
>                VLC_ADD_LIBS([zvbi],[ -lpthreadGC2])
>            fi
> -          VLC_ADD_CFLAGS([zvbi],[$ZVBI_CFLAGS])
> +          VLC_ADD_CFLAGS([zvbi v4l2],[$ZVBI_CFLAGS])
>            VLC_ADD_PLUGIN([zvbi])
>            AC_DEFINE(ZVBI_COMPILED, 1, [Define if the zvbi module is

You don't need to patch configure.ac and CFLAGS won't work as is. See below.

> built]) AS_IF( [test "${enable_telx}" = "yes"],[
> diff --git a/modules/access/Modules.am b/modules/access/Modules.am
> index 5665477..764472d 100644
> --- a/modules/access/Modules.am
> +++ b/modules/access/Modules.am
> @@ -132,6 +132,7 @@ libv4l2_plugin_la_SOURCES = \
>  	v4l2/videodev2.h \
>  	v4l2/v4l2.c \
>  	v4l2/video.c \
> +	v4l2/vbi.c \
>  	v4l2/demux.c \
>  	v4l2/access.c \
>  	v4l2/radio.c \
> @@ -139,7 +140,7 @@ libv4l2_plugin_la_SOURCES = \
>  	v4l2/lib.c \
>  	v4l2/v4l2.h
>  libv4l2_plugin_la_CFLAGS = $(AM_CFLAGS)

Missing $(ZVBI_CFLAGS), I think

> -libv4l2_plugin_la_LIBADD = $(AM_LIBADD) $(LIBDL) $(LIBM)
> +libv4l2_plugin_la_LIBADD = $(AM_LIBADD) $(LIBDL) $(LIBM) $(LIBS_v4l2)

Could use $(ZVBI_LIBS) instead.

>  if HAVE_V4L2
>  libvlc_LTLIBRARIES += libv4l2_plugin.la
>  endif
> diff --git a/modules/access/v4l2/demux.c b/modules/access/v4l2/demux.c
> index 15b540f..21e1709 100644
> --- a/modules/access/v4l2/demux.c
> +++ b/modules/access/v4l2/demux.c
> @@ -58,6 +58,14 @@ struct demux_sys_t
>      es_out_id_t *es;
>      vlc_v4l2_ctrl_t *controls;
>      mtime_t start;
> +
> +#ifdef ZVBI_COMPILED
> +    vbi_capture *vbi_cap;
> +    es_out_id_t *p_es_subt[4];
> +    uint8_t* vbi_raw;
> +    vbi_sliced* vbi_sliced;
> +    unsigned int vbi_sliced_size;
> +#endif
>  };
> 
>  static void *UserPtrThread (void *);
> @@ -65,6 +73,9 @@ static void *MmapThread (void *);
>  static void *ReadThread (void *);
>  static int DemuxControl( demux_t *, int, va_list );
>  static int InitVideo (demux_t *, int fd, uint32_t caps);
> +#ifdef ZVBI_COMPILED
> +static int InitVBI (demux_t *);
> +#endif
> 
>  int DemuxOpen( vlc_object_t *obj )
>  {
> @@ -88,6 +99,17 @@ int DemuxOpen( vlc_object_t *obj )
>          goto error;
>      sys->fd = fd;
> 
> +#ifdef ZVBI_COMPILED
> +    char *vbi_path = var_InheritString (obj, CFG_PREFIX"vbidev");

I gather there is no way to deduce the VBI node, if any, from the video node, 
is there? I fear that most users will fail to enable VBI manually :-(

> +    if (vbi_path != NULL)
> +    {
> +        sys->vbi_cap = OpenVBIDev (obj, vbi_path);
> +        if (sys->vbi_cap)
> +            InitVBI(demux);
> +        free(vbi_path);
> +    }
> +#endif
> +

IMHO, all this VBI code and VBI data should be initialized in vbi.c in 
OpenVBIDev() kinda like control.c. Then there would be a single function call 
here. That way the overhead in non-VBI case is reduced to a single NULL 
pointer also.

>      if (InitVideo (demux, fd, caps))
>      {
>          v4l2_close (fd);
> @@ -448,6 +470,42 @@ static int InitVideo (demux_t *demux, int fd, uint32_t
> caps) return 0;
>  }
> 
> +#ifdef ZVBI_COMPILED
> +static int InitVBI (demux_t *demux)
> +{
> +    int i;
> +    demux_sys_t *sys = demux->p_sys;
> +    vbi_raw_decoder *par;
> +
> +    par = vbi_capture_parameters (sys->vbi_cap);
> +    if (!par)
> +    {
> +        msg_Err( demux, "Cannot get vbi capture parameters");
> +    } else {
> +        unsigned int src_w = par->bytes_per_line / 1;
> +        unsigned int src_h = par->count[0] + par->count[1];
> +        sys->vbi_raw = calloc (1, src_w * src_h);

Either deal with error cases, or use xcalloc() if you don't mind crashing.

Do you really need to zero the table? If not, use (x)malloc().

> +        sys->vbi_sliced_size = sizeof (vbi_sliced) * src_h;
> +        sys->vbi_sliced = malloc (sys->vbi_sliced_size);

Same as above.

> +    }
> +
> +    for (i = 0; i < 4; i++)
> +    {
> +        es_format_t fmt;
> +        char buf[5];
> +
> +        es_format_Init( &fmt, SPU_ES, VLC_FOURCC('c','c',0x31 + i,' ') );
> +        snprintf(buf, sizeof(buf), "CC%d", i + 1);
> +        fmt.psz_description = strdup(buf);

Use asprintf().

> +        msg_Dbg( demux, "new spu es %4.4s", (char*)&fmt.i_codec );
> +
> +        sys->p_es_subt[i] = es_out_Add( demux->out, &fmt );
> +    }
> +
> +    return 0;
> +}
> +#endif
> +
>  void DemuxClose( vlc_object_t *obj )
>  {
>      demux_t *demux = (demux_t *)obj;
> @@ -459,6 +517,16 @@ void DemuxClose( vlc_object_t *obj )
>          StopMmap (sys->fd, sys->bufv, sys->bufc);
>      ControlsDeinit( obj, sys->controls );
>      v4l2_close (sys->fd);
> +
> +#ifdef ZVBI_COMPILED
> +    if( sys->vbi_cap )
> +    {
> +        vbi_capture_delete( sys->vbi_cap );
> +    }
> +    free( sys->vbi_raw );
> +    free( sys->vbi_sliced );

Hmm, did you try under valgrind? I think this may be uninitialized. Maybe a 
CloseVBI() function or something...

> +#endif
> +
>      free( sys );
>  }
> 
> @@ -551,31 +619,70 @@ static void *MmapThread (void *data)
>      demux_t *demux = data;
>      demux_sys_t *sys = demux->p_sys;
>      int fd = sys->fd;
> -    struct pollfd ufd[1];
> +    struct pollfd ufd[2];
> +    nfds_t numfds = 1;
> 
>      ufd[0].fd = fd;
>      ufd[0].events = POLLIN;
> 
> +#ifdef ZVBI_COMPILED
> +    if ( sys->vbi_cap )
> +    {
> +        ufd[1].fd = vbi_capture_fd(sys->vbi_cap);
> +        ufd[1].events = POLLIN;
> +        ufd[1].revents = 0;
> +        numfds++;
> +
> +        /* Do a single read and throw away the results so that ZVBI calls
> +           the STREAMON ioctl() */
> +        GrabVBI( VLC_OBJECT(demux), sys->vbi_cap, sys->vbi_sliced,
> +                 sys->vbi_sliced_size, sys->vbi_raw );

I'm not sure I understand. You want to force zvbi to start streaming before 
poll()? Shouldn't that be done in Open()?

> +    }
> +#endif
> +
>      for (;;)
>      {
>          /* Wait for data */
> -        if (poll (ufd, 1, -1) == -1)
> +        if (poll (ufd, numfds, -1) == -1)
>          {
>             if (errno != EINTR)
>                 msg_Err (demux, "poll error: %m");
>             continue;
>          }
> 
> -        int canc = vlc_savecancel ();
> -        block_t *block = GrabVideo (VLC_OBJECT(demux), fd, sys->bufv);
> -        if (block != NULL)
> +        if( ufd[0].revents & POLLIN )
>          {
> -            block->i_pts = block->i_dts = mdate ();
> -            block->i_flags |= sys->block_flags;
> -            es_out_Control (demux->out, ES_OUT_SET_PCR, block->i_pts);
> -            es_out_Send (demux->out, sys->es, block);
> +            int canc = vlc_savecancel ();
> +            block_t *block = GrabVideo (VLC_OBJECT(demux), fd, sys->bufv);
> +            if (block != NULL)
> +            {
> +                block->i_pts = block->i_dts = mdate ();
> +                block->i_flags |= sys->block_flags;
> +                es_out_Control (demux->out, ES_OUT_SET_PCR, block->i_pts);
> +                es_out_Send (demux->out, sys->es, block);
> +            }
> +            vlc_restorecancel (canc);
>          }
> -        vlc_restorecancel (canc);
> +#ifdef ZVBI_COMPILED
> +        if( sys->vbi_cap && (ufd[1].revents & POLLIN) )
> +        {
> +            block_t *p_block =  GrabVBI( VLC_OBJECT(demux), sys->vbi_cap,
> +                                         sys->vbi_sliced,
> sys->vbi_sliced_size, +                                        
> sys->vbi_raw );
> +            if( p_block )
> +            {
> +                int i;
> +                for (i = 0; i < 4; i++)
> +                {
> +                    if (i == 3)
> +                        es_out_Send( demux->out, sys->p_es_subt[i],
> p_block ); +                    else
> +                        es_out_Send( demux->out, sys->p_es_subt[i],
> +                                     block_Duplicate(p_block) );

block_Duplicate() can fail. You could unroll the last iteration.

> +                }
> +            }
> +        }
> +#endif

Doesn't this needs to vlc_(save|restore)cancel also?
Also, considering the next patch (read/userptr), it might make sense to factor 
some of this code out into vbi.c...

>      }
> 
>      assert (0);
> diff --git a/modules/access/v4l2/v4l2.c b/modules/access/v4l2/v4l2.c
> index 878fb81..989a3c3 100644
> --- a/modules/access/v4l2/v4l2.c
> +++ b/modules/access/v4l2/v4l2.c
> @@ -43,6 +43,10 @@
> 
>  #define VIDEO_DEVICE_TEXT N_( "Video capture device" )
>  #define VIDEO_DEVICE_LONGTEXT N_("Video capture device node." )
> +#define VBI_DEVICE_TEXT N_("VBI capture device")
> +#define VBI_DEVICE_LONGTEXT N_( \
> +    "The device node where VBI data can be read "   \
> +    " (for closed captions) " )
>  #define STANDARD_TEXT N_( "Standard" )
>  #define STANDARD_LONGTEXT N_( \
>      "Video standard (Default, SECAM, PAL, or NTSC)." )
> @@ -277,6 +281,10 @@ vlc_module_begin ()
>      add_loadfile( CFG_PREFIX "dev", "/dev/video0",
>                    VIDEO_DEVICE_TEXT, VIDEO_DEVICE_LONGTEXT, false )
>          change_safe()
> +#ifdef ZVBI_COMPILED
> +    add_loadfile( CFG_PREFIX "vbidev", NULL,
> +                  VBI_DEVICE_TEXT, VBI_DEVICE_LONGTEXT, false )
> +#endif
>      add_string( CFG_PREFIX "standard", "",
>                  STANDARD_TEXT, STANDARD_LONGTEXT, false )
>          change_string_list( standards_vlc, standards_user )
> @@ -502,6 +510,44 @@ int OpenDevice (vlc_object_t *obj, const char *path,
> uint32_t *restrict caps) return fd;
>  }
> 
> +
> /*************************************************************************
> **** +  * OpenVBIDev:
> + 
> **************************************************************************
> ***/ +#ifdef ZVBI_COMPILED
> +vbi_capture* OpenVBIDev( vlc_object_t *p_obj, const char *psz_device)
> +{
> +    vbi_capture *cap;
> +    char* errstr = NULL;
> +
> +    //Can put more in here. See osd.c in zvbi package.
> +    unsigned int services = VBI_SLICED_CAPTION_525;
> +
> +    cap = vbi_capture_v4l2_new (psz_device,
> +                                /* buffers */ 5,
> +                                &services,
> +                                /* strict */ 1,
> +                                &errstr,
> +                                /* verbose */ 1);
> +    if (cap == NULL)
> +    {
> +        msg_Err( p_obj, "Cannot capture vbi data with v4l2 interface
> (%s)", errstr ); +    }
> +#if defined( HAVE_FCNTL )

So my first thought was, you don't need this #ifdef because any system with V4L 
has fcntl(). But then I realized the code would not quite work anyway.

The VBI node needs to be open()ed with the O_CLOEXEC flag set. Otherwise, the 
file descriptor can be leaked into another forking thread, before the fcntl() 
system call. I had the same problem with ALSA-lib already; I sent a patch.

An alternative is to open the file descriptor in VLC and pass the descriptor to 
the library, as with libv4l. I doubt libzvbi supports that though.

> +    else
> +    {
> +        /* We need to ensure that the /dev/vbiX filehandle doesn't get
> +           inherited by child processes that might get spawned.  In
> particular, +           the screensaver keepalive will inherit the handle,
> which will +           prevent it from being closed properly */
> +        int fd = vbi_capture_fd(cap);
> +        fcntl( fd, F_SETFD, fcntl( fd, F_GETFD ) | FD_CLOEXEC );
> +    }
> +#endif
> +
> +    return cap;
> +}
> +#endif
> +
>  v4l2_std_id var_InheritStandard (vlc_object_t *obj, const char *varname)
>  {
>      char *name = var_InheritString (obj, varname);
> diff --git a/modules/access/v4l2/v4l2.h b/modules/access/v4l2/v4l2.h
> index fa0e4e7..97f0764 100644
> --- a/modules/access/v4l2/v4l2.h
> +++ b/modules/access/v4l2/v4l2.h
> @@ -20,6 +20,10 @@
> 
>  #include "videodev2.h"
> 
> +#ifdef ZVBI_COMPILED
> +#   include <libzvbi.h>
> +#endif
> +
>  /* libv4l2 functions */
>  extern int v4l2_fd_open (int, int);
>  extern int (*v4l2_close) (int);
> @@ -41,6 +45,9 @@ struct buffer_t
>  /* v4l2.c */
>  void ParseMRL(vlc_object_t *, const char *);
>  int OpenDevice (vlc_object_t *, const char *, uint32_t *);
> +#ifdef ZVBI_COMPILED
> +vbi_capture* OpenVBIDev( vlc_object_t *, const char *);
> +#endif
>  v4l2_std_id var_InheritStandard (vlc_object_t *, const char *);
> 
>  /* video.c */
> @@ -57,6 +64,13 @@ void StopMmap (int, struct buffer_t *, uint32_t);
> 
>  block_t* GrabVideo (vlc_object_t *, int, const struct buffer_t *);
> 
> +#ifdef ZVBI_COMPILED
> +/* vbi.c */
> +block_t* GrabVBI (vlc_object_t *p_demux, vbi_capture *vbi_cap,
> +                  vbi_sliced *sliced, unsigned int vbi_sliced_size,
> +                  uint8_t *vbi_raw);
> +#endif
> +
>  /* demux.c */
>  int DemuxOpen(vlc_object_t *);
>  void DemuxClose(vlc_object_t *);
> diff --git a/modules/access/v4l2/vbi.c b/modules/access/v4l2/vbi.c
> new file mode 100644
> index 0000000..18c1482
> --- /dev/null
> +++ b/modules/access/v4l2/vbi.c
> @@ -0,0 +1,86 @@
> +/*************************************************************************
> **** + * vbi.c : Video4Linux2 VBI input module for vlc
> +
> **************************************************************************
> *** + * Copyright (C) 2002-2009 the VideoLAN team
> + *
> + * Author: Devin Heitmueller <dheitmueller at kernellabs dot 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 Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> License + * along with this program; if not, write to the Free Software
> Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston MA
> 02110-1301, USA. +
> **************************************************************************
> ***/ +
> +#ifdef HAVE_CONFIG_H
> +# include "config.h"
> +#endif
> +
> +#include <errno.h>
> +#include <vlc_common.h>
> +#include <vlc_block.h>
> +
> +#include "v4l2.h"
> +
> +#ifdef ZVBI_COMPILED
> +block_t *GrabVBI( vlc_object_t *p_demux, vbi_capture *vbi_cap,
> +                  vbi_sliced *sliced, unsigned int vbi_sliced_size,
> +                  uint8_t *vbi_raw)
> +{
> +    block_t     *p_block=NULL;
> +
> +    struct timeval timeout={0,0}; /* poll */
> +    double timestamp;
> +    int n_lines;
> +
> +    memset(sliced, 0, vbi_sliced_size);
> +
> +    int r = vbi_capture_read (vbi_cap,
> +                              vbi_raw,
> +                              sliced,
> +                              &n_lines,
> +                              &timestamp,
> +                              &timeout);
> +    switch (r) {
> +        case -1:
> +            msg_Err( p_demux, "Error reading VBI (%s)", strerror(errno) );

strerror() is not reentrant. Use %m (recommended) or strerror_r() (not 
recommended).

> +            break;
> +        case  0: /* nothing avail */
> +            break;
> +        case  1: /* got data */
> +            if (n_lines)
> +            {
> +                int sliced_size = 2; /* Number of sliced lines decoded
> from raw */ +                int size = (sliced_size + 1) * n_lines;
> +                if( !( p_block = block_New( p_demux, size ) ) )

Use block_Alloc(). block_New() is legacy.

> +                {
> +                    msg_Err( p_demux, "cannot get block" );
> +                }
> +                else
> +                {
> +                    int field;
> +                    uint8_t* data = p_block->p_buffer;
> +                    for (field=0; field<n_lines; field++)
> +                    {
> +                        *data = field;
> +                        data++;
> +                        memcpy(data, (sliced+field)->data, sliced_size);
> +                        data += sliced_size;
> +                    }
> +                    data = p_block->p_buffer;

That instruction has no effects, does it?

> +                    p_block->i_buffer = size;
> +                    p_block->i_pts = p_block->i_pts = mdate();
> +                }
> +            }
> +    }
> +    return p_block;
> +}
> +#endif

-- 
Rémi Denis-Courmont
http://www.remlab.net/



More information about the vlc-devel mailing list