[vlc-devel] [RFC] SDI input module

[Rémi Duraffort]

Hello,
only some cosmetics remarks.


> diff --git a/modules/access/Modules.am b/modules/access/Modules.am
> [...]
> +#include <vlc/vlc.h>
> +#include <vlc_input.h>
> +#include <vlc_access.h>
> +#include <vlc_demux.h>
> +
> +#include "sdi.h"
<sdi.h> or the sdi.h file is missing in the patch ?


> +/*****************************************************************************
> + * DemuxOpen:
> + *****************************************************************************/
> +static int DemuxOpen( vlc_object_t *p_this )
> +{
> +    demux_t     *p_demux = (demux_t *)p_this;
> +    demux_sys_t *p_sys;
> +    vlc_value_t val;
> +    char        *psz_parser;
> +    int         i;
> +
> +    /* Fill p_demux field */
> +    p_demux->pf_demux = DemuxDemux;
> +    p_demux->pf_control = DemuxControl;
> +    p_demux->p_sys = p_sys = malloc( sizeof( demux_sys_t ) );
> +    memset( p_sys, 0, sizeof( demux_sys_t ) );
calloc

> +
> +    p_sys->i_state = STATE_NOSYNC;
> +    p_sys->i_last_state_change = mdate();
> +
> +    var_Create( p_demux, "sdi-aspect-ratio",
> +                VLC_VAR_STRING|VLC_VAR_DOINHERIT );
> +    var_Get( p_demux, "sdi-aspect-ratio", &val );
var_CreateGetString or var_InheritString (I think var_Inherit is better
here).

> +    psz_parser = strchr( val.psz_string, ':' );
> +    if ( psz_parser )
> +    {
> +        *psz_parser++ = '\0';
> +        p_sys->i_forced_aspect = p_sys->i_aspect =
> +            strtol( val.psz_string, NULL, 0 ) * VOUT_ASPECT_FACTOR
> +             / strtol( psz_parser, NULL, 0 );
> +    }
> +    else
> +        p_sys->i_forced_aspect = 0;
> +    free( val.psz_string );
> +
> +    var_Create( p_demux, "sdi-id-video",
> +                VLC_VAR_INTEGER|VLC_VAR_DOINHERIT );
> +    var_Get( p_demux, "sdi-id-video", &val );
var_Inherit

> +    p_sys->i_id_video = val.i_int;
> +
> +    /* Audio ES */
> +    var_Create( p_demux, "sdi-audio",
> +                VLC_VAR_STRING|VLC_VAR_DOINHERIT );
> +    var_Get( p_demux, "sdi-audio", &val );
var_Inherit

> +    /* Teletext ES */
> +    var_Create( p_demux, "sdi-telx",
> +                VLC_VAR_STRING|VLC_VAR_DOINHERIT );
> +    var_Get( p_demux, "sdi-telx", &val );
> +    p_sys->psz_telx = val.psz_string;
var_Inherit

> +
> +    var_Create( p_demux, "sdi-telx-lang",
> +                VLC_VAR_STRING|VLC_VAR_DOINHERIT );
> +    var_Get( p_demux, "sdi-telx-lang", &val );
> +    p_sys->psz_telx_lang = val.psz_string;
var_Inherit


> +static int Open( vlc_object_t *p_this )
> +{
> +    var_Create( p_demux, "sdi-link", VLC_VAR_INTEGER|VLC_VAR_DOINHERIT );
> +    var_Get( p_demux, "sdi-link", &val );
> +    p_sys->i_link = val.i_int;
var_Inherit


> +/*****************************************************************************
> + * Low-level device stuff
> + *****************************************************************************/
> +#define MAXLEN 256
> +
> +static int ReadULSysfs( const char *psz_fmt, unsigned int i_link )
> +{
> +    char psz_file[MAXLEN], psz_data[MAXLEN];
> +    char *psz_tmp;
> +    int i_fd;
> +    ssize_t i_ret;
> +    unsigned int i_data;
> +
> +    snprintf( psz_file, sizeof(psz_file), psz_fmt, i_link );
> +    psz_file[sizeof(psz_file) - 1] = '\0';
AFAIR, snprintf allways add a '\0' at the end of the string


> +static ssize_t WriteULSysfs( const char *psz_fmt, unsigned int i_link,
> +                             unsigned int i_buf )
> +{
> +    char psz_file[MAXLEN], psz_data[MAXLEN];
> +    int i_fd;
> +    ssize_t i_ret;
> +
> +    snprintf( psz_file, sizeof(psz_file), psz_fmt, i_link );
> +    psz_file[sizeof(psz_file) - 1] = '\0';
> +
> +    snprintf( psz_data, sizeof(psz_data), "%u\n", i_buf );
> +    psz_file[sizeof(psz_data) - 1] = '\0';
Same here.


> +static int InitCapture( demux_t *p_demux )
> +{
> [...]
> +    snprintf( psz_dev, sizeof(psz_dev), SDI_DEVICE, p_sys->i_link );
> +    psz_dev[sizeof(psz_dev) - 1] = '\0';
Same


-- 
Rémi Duraffort | ivoire