[vlc-devel] [PATCH] Added libvlc_video_take_snapshot_addr to libvlc and related functions to libvlccore

[Laurent Aimar]

Hi,

On Mon, Nov 28, 2011 at 10:22:54PM +0100, PaulK wrote:
> +size_t
> +libvlc_video_snapshot_get_data( libvlc_media_player_t *p_mi, unsigned num,
> +                                 void **psz_addr, unsigned int i_width, 
> +                                 unsigned int i_height )
> +{
> +    picture_t *picture_buffer;
> +    block_t *block_buffer;
> +    size_t buffer_len = 0;
> +    char *buffer = NULL;
> +    video_format_t fmt;
> +
> +    vout_thread_t *p_vout = GetVout( p_mi, num );
> +    if (p_vout == NULL)
> +        return -1;
> +
> +    if( vout_GetSnapshot( p_vout, &block_buffer, &picture_buffer, &fmt, "png", 500*1000 ) )
 As you don't use picture_buffer, simply use NULL, this way you won't
have to release it.
> +    {
> +        picture_buffer = NULL;
> +        block_buffer = NULL;
 Useless assignment.
> +        return -1;
> +    }
> +
> +    if( block_buffer == NULL )
> +        return -1;
 The test is not needed, vout_GetSnapshot won't return sucess unless
block_buffer is not NULL.
> +
> +    buffer_len = block_buffer->i_buffer;
> +    if( buffer_len <= 0 )
> +        return -1;
 Memory leak.
> +
> +    buffer = malloc( buffer_len );
> +    if( buffer == NULL )
> +        return -1;
 Same here.
> +
> +    psz_addr[0] = buffer;
> +    memcpy( buffer, block_buffer->p_buffer, buffer_len );
> +
> +    if( picture_buffer != NULL )
> +        picture_Release( picture_buffer );
> +    if( block_buffer != NULL )
 Useless test.
> +        block_Release( block_buffer );
> +
> +    return buffer_len;
> +}
 Also you cannot return -1 and use size_t for the return type. You should use
ssize_t for a signed version.

Regards,

-- 
fenrir