[vlc-devel] dc1394 module update

[Jean-Baptiste Kempf]

On Thu, Oct 15, 2009 at 11:25:15PM +0200, Frédéric BENOIST wrote :
> I updated  the dc1394 module for libdc1394 2.1.2.
> I based my work on a previous patch written by Rob Shorrt:
> http://mailman.videolan.org/pipermail/vlc-devel/2007-May/031606.html

> diff --git a/vlc-1.0.1/configure.ac b/configure.ac
Ok.

> diff --git a/vlc-1.0.1/modules/access/dc1394.c b/dc1394.c
> index 9fa05aa..3ff06af 100644
> --- a/vlc-1.0.1/modules/access/dc1394.c
> +++ b/dc1394.c

> +        for( i=0; i < (int)sys->num_cameras; i++ )

You can (and probably should) use:
for( int i = 0; i < (int)sys->num_cameras; i++ )

>  /*****************************************************************************
> @@ -212,14 +208,10 @@ static int Open( vlc_object_t *p_this )
>      p_demux->p_sys = p_sys = calloc( 1, sizeof( demux_sys_t ) );
>      if( !p_sys )
> +    {
> +        msg_Err( p_demux, "not enough memory available");
>          return VLC_ENOMEM;
> +    }
I think that throwing a msg_Err when ENOMEM is wrong and useless...

> +        msg_Err( p_demux, "unable to print camera info");
"Unable" maybe.

> +        msg_Err( p_demux, "unable to set framerate");
ditto

> +        msg_Err( p_demux, "unable to set video mode");
And again.

> +            msg_Err( p_demux ,"%d", res );
> +            msg_Err( p_demux ,"unable to setup capture" );
> +            msg_Err( p_demux, "make sure that the supplied video mode, framerate,");
> +            msg_Err( p_demux, "and format are supported by your camera");
Do we need all that debug?

> +    if( psz_dup ) free( psz_dup );
Useless if

In general:
 - be careful about your trailing spaces
 - be sure there is not too many msg_Err or other left over debug
 - try to break Long lines when you need and check style :)

Else, thanks A LOT for this work, I hope we can merge it quite soon,
because that module was worrying me...

Best Regards,

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/