[vlc-devel] [PATCH] Rawaudio input demux (version 3)

[Laurent Aimar]

Hi,

On Mon, May 04, 2009, Jarmo Torvinen wrote:
> Thanks for the very insighted review. I think I now understand a bit 
> better of the VLC internals.
> Anyway, please find attached the third version of the raw audio demux 
> module. This fixes the issues highlighted by fenfir and additionally
> adds checks for sensible channels and samplerate parameters.
 There is a few more ;)

> +    char *psz_fourcc = var_CreateGetString( p_demux, "rawaud-fourcc" );
> +
> +    if( psz_fourcc == NULL )
> +    {
> +        msg_Err( p_demux, "null rawaud fourcc defined");
> +        free( p_sys );
> +        return VLC_EGENERIC;
> +    }
> +
> +    if( strlen( psz_fourcc ) != 4 )
> +    {
> +        msg_Err( p_demux, "parameter rawaud-fourcc must be 4 characters long");
> +        free( p_sys );
> +        return VLC_EGENERIC;
> +    }
 You need to free psz_fourcc here.
 You could also merge both tests (free(NULL) is valid in VLC) but
that's up to your preferences.

> +    p_sys->fmt.psz_language = var_CreateGetString( p_demux, "rawaud-lang" );
> +    p_sys->fmt.audio.i_channels = var_CreateGetInteger( p_demux, "rawaud-channels" );
> +    p_sys->fmt.audio.i_rate = var_CreateGetInteger( p_demux, "rawaud-samplerate" );
> +
> +    if( p_sys->fmt.audio.i_rate <= 0 )
> +    {
> +      msg_Err( p_demux, "invalid sample rate");
> +      free( p_sys->fmt.psz_language);
 es_format_Clean is a better way (and will avoid leaks if es_format_Init
allocates any memory in the future).

> +    if( p_sys->fmt.audio.i_channels <= 0 )
> +    {
> +      msg_Err( p_demux, "invalid number of channels");
> +      free( p_sys->fmt.psz_language);
 Same here.

> +    if( p_sys->fmt.psz_language == NULL )
> +    {
> +      msg_Err( p_demux, "invalid language");
 You could also not detect this one. The core will happily accept a empty or
NULL language (it will means undefined, which is perfectly valid).

> +    /* add the es */
> +/*****************************************************************************
> + * Close: frees unused data
> + *****************************************************************************/
> +static void Close( vlc_object_t *p_this )
> +{
> +    demux_t     *p_demux = (demux_t*)p_this;
> +    demux_sys_t *p_sys  = p_demux->p_sys;
> +
> +    free( p_sys->fmt.psz_language );
 Same here (es_format_Clean).

> +/*****************************************************************************
> + * Control:
> + *****************************************************************************/
> +static int Control( demux_t *p_demux, int i_query, va_list args )
> +{
> +    demux_sys_t *p_sys  = p_demux->p_sys;
> +
> +    return demux_vaControlHelper( p_demux->s, 0, -1,
> +                                   p_sys->fmt.i_bitrate, 1, i_query, args );
> +}
 The remark about this function remains (Seek will be randomly broken).

-- 
fenrir