[vlc-devel] [PATCH] Added hotkey for video transformation. Pressing Ctrl+Alt+r the user can navigate through the available transformation modes.

Rémi Denis-Courmont remi at remlab.net
Sun Jan 19 03:13:30 CET 2014


On Sat, 18 Jan 2014 18:22:04 +0200, Stefanos Orovas
<stef.orovas at gmail.com>
wrote:
> diff --git a/modules/control/hotkeys.c b/modules/control/hotkeys.c
> index ab68340..51ad890 100644
> --- a/modules/control/hotkeys.c
> +++ b/modules/control/hotkeys.c
> @@ -836,7 +836,50 @@ static int PutAction( intf_thread_t *p_intf, int
> i_action )
>              if( p_vout )
>                  var_DecInteger( p_vout, "crop-right" );
>              break;
> -
> +	case ACTIONID_TRANSFORM:
> +	    if ( p_vout )
> +	    {
> +		char *psz_parser, *transform_type = "";
> +		char *psz_string = config_GetPsz( p_vout, "video-filter" );

I believe that won't work properly if video-filter was passed via the
command line.

> +		static int tr_mode = 0;

NEVER use read/write static data (unless you REALLY know what you are
doing).

> +
> +		if ( !psz_string ) psz_string = strdup( "" );
> +		psz_parser = strstr( psz_string, "transform" );

This will not work if the parameters of another video filter contain the
substring "transform", which is in fact totally possible, since some
parameters are arbitary strings.

> +
> +		if ( !psz_parser ){
> +		    psz_parser = psz_string;
> +		    asprintf( &psz_string, ( *psz_string ) ? "%s:%s" : "%s%s",
> psz_string, "transform" );
> +		}
> +
> +		switch( tr_mode ){
> +	 	    case 0: transform_type = "90"; 	      break;
> +		    case 1: transform_type = "180"; 	      break;
> +		    case 2: transform_type = "270"; 	      break;
> +		    case 3: transform_type = "hflip";         break;
> +		    case 4: transform_type = "vflip";         break;
> +		    case 5: transform_type = "transpose";     break;
> +		    case 6: transform_type = "antitranspose"; break;
> +		    case 7: // reset psz_string  
> +		        if( *( psz_parser + strlen( "transform" ) ) == ':' )
> +			    memmove( psz_parser, psz_parser + strlen( "transform" ) + 1,
> +			          	 strlen( psz_parser + strlen( "transform" ) + 1 ) + 1 );
> +			else *psz_parser = '\0';

I am not sure I follow the logic, but this looks like a heap buffer
overflow to me.

> +
> +			/* Remove trailing : : */
> +			size_t i_len = strlen( psz_string );
> +			if( i_len > 0 && *( psz_string + i_len - 1 ) == ':' )
> +			{
> +			    *( psz_string + i_len - 1 ) = '\0';
> +			}
> +			break;
> +		}
> +		tr_mode ++; tr_mode = tr_mode % 8;
> +
> +		config_PutPsz( p_intf, "transform-type", transform_type );
> +		config_PutPsz( p_intf, "video-filter", psz_string );

As pointed out previously, this would clobber the user configuration.
Please stick to var_*().

> +		var_SetString( p_vout, "video-filter", psz_string );

Variables go out of scope. Memory leaks...

> +	    }
> +	    break;
>           case ACTIONID_TOGGLE_AUTOSCALE:
>              if( p_vout )
>              {
-- 
Rémi Denis-Courmont
Sent from my collocated server



More information about the vlc-devel mailing list