From: Henrik Gramner Date: Thu, 4 Feb 2016 19:06:57 +0000 (+0100) Subject: cli: Refactor filter option parsing X-Git-Url: https://git.sesse.net/?a=commitdiff_plain;h=0082b717199bafb4abbb6638e7c30d50deaf2c1b;hp=dfe394cadc8a39752de5b3f4a0be222c1b9290f2;p=x264 cli: Refactor filter option parsing The old code contained a whole bunch of memory leaks, unchecked mallocs, sections of dead code, etc. and was generally overly complex. Also consolidate some memory allocations into a single one. --- diff --git a/filters/filters.c b/filters/filters.c index aecd296b..204dda53 100644 --- a/filters/filters.c +++ b/filters/filters.c @@ -5,6 +5,7 @@ * * Authors: Diogo Franco * Steven Walters + * Henrik Gramner * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -27,159 +28,95 @@ #include "filters.h" #define RETURN_IF_ERROR( cond, ... ) RETURN_IF_ERR( cond, "options", NULL, __VA_ARGS__ ) -char **x264_split_string( char *string, char *sep, int limit ) +char **x264_split_options( const char *opt_str, const char * const *options ) { - if( !string ) + int opt_count = 0, options_count = 0, found_named = 0, size = 0; + const char *opt = opt_str; + + if( !opt_str ) return NULL; - int sep_count = 0; - int sep_len = strlen( sep ); - char *tmp = string; - while( ( tmp = ( tmp = strstr( tmp, sep ) ) ? tmp + sep_len : 0 ) ) - ++sep_count; - if( sep_count == 0 ) - { - if( string[0] == '\0' ) - return calloc( 1, sizeof( char* ) ); - char **ret = calloc( 2, sizeof( char* ) ); - ret[0] = strdup( string ); - return ret; - } - char **split = calloc( ( limit > 0 ? limit : sep_count ) + 2, sizeof(char*) ); - int i = 0; - char *str = strdup( string ); - assert( str ); - char *esc = NULL; - char *tok = str, *nexttok = str; + while( options[options_count] ) + options_count++; + do { - nexttok = strstr( nexttok, sep ); - if( nexttok ) - *nexttok++ = '\0'; - if( ( limit > 0 && i >= limit ) || - ( i > 0 && ( ( esc = strrchr( split[i-1], '\\' ) ) ? esc[1] == '\0' : 0 ) ) ) // Allow escaping + int length = strcspn( opt, "=," ); + if( opt[length] == '=' ) { - int j = i-1; - if( esc ) - esc[0] = '\0'; - split[j] = realloc( split[j], strlen( split[j] ) + sep_len + strlen( tok ) + 1 ); - assert( split[j] ); - strcat( split[j], sep ); - strcat( split[j], tok ); - esc = NULL; + const char * const *option = options; + while( *option && (strlen( *option ) != length || strncmp( opt, *option, length )) ) + option++; + + RETURN_IF_ERROR( !*option, "Invalid option '%.*s'\n", length, opt ) + found_named = 1; + length += strcspn( opt + length, "," ); } else { - split[i++] = strdup( tok ); - assert( split[i-1] ); + RETURN_IF_ERROR( opt_count >= options_count, "Too many options given\n" ) + RETURN_IF_ERROR( found_named, "Ordered option given after named\n" ) + size += strlen( options[opt_count] ) + 1; } - tok = nexttok; - } while ( tok ); - free( str ); - assert( !split[i] ); + opt_count++; + opt += length; + } while( *opt++ ); - return split; -} + int offset = 2 * (opt_count+1) * sizeof(char*); + size += offset + (opt - opt_str); + char **opts = calloc( 1, size ); + RETURN_IF_ERROR( !opts, "malloc failed\n" ) -void x264_free_string_array( char **array ) -{ - if( !array ) - return; - for( int i = 0; array[i] != NULL; i++ ) - free( array[i] ); - free( array ); -} +#define insert_opt( src, length )\ +do {\ + opts[i++] = memcpy( (char*)opts + offset, src, length );\ + offset += length + 1;\ + src += length + 1;\ +} while( 0 ) -char **x264_split_options( const char *opt_str, const char *options[] ) -{ - if( !opt_str ) - return NULL; - char *opt_str_dup = strdup( opt_str ); - char **split = x264_split_string( opt_str_dup, ",", 0 ); - free( opt_str_dup ); - int split_count = 0; - while( split[split_count] != NULL ) - ++split_count; - - int options_count = 0; - while( options[options_count] != NULL ) - ++options_count; - - char **opts = calloc( split_count * 2 + 2, sizeof( char * ) ); - char **arg = NULL; - int opt = 0, found_named = 0, invalid = 0; - for( int i = 0; split[i] != NULL; i++, invalid = 0 ) + for( int i = 0; i < 2*opt_count; ) { - arg = x264_split_string( split[i], "=", 2 ); - if( arg == NULL ) + int length = strcspn( opt_str, "=," ); + if( opt_str[length] == '=' ) { - if( found_named ) - invalid = 1; - else RETURN_IF_ERROR( i > options_count || options[i] == NULL, "Too many options given\n" ) - else - { - opts[opt++] = strdup( options[i] ); - opts[opt++] = strdup( "" ); - } - } - else if( arg[0] == NULL || arg[1] == NULL ) - { - if( found_named ) - invalid = 1; - else RETURN_IF_ERROR( i > options_count || options[i] == NULL, "Too many options given\n" ) - else - { - opts[opt++] = strdup( options[i] ); - if( arg[0] ) - opts[opt++] = strdup( arg[0] ); - else - opts[opt++] = strdup( "" ); - } + insert_opt( opt_str, length ); + length = strcspn( opt_str, "," ); } else { - found_named = 1; - int j = 0; - while( options[j] != NULL && strcmp( arg[0], options[j] ) ) - ++j; - RETURN_IF_ERROR( options[j] == NULL, "Invalid option '%s'\n", arg[0] ) - else - { - opts[opt++] = strdup( arg[0] ); - opts[opt++] = strdup( arg[1] ); - } + const char *option = options[i/2]; + int option_length = strlen( option ); + insert_opt( option, option_length ); } - RETURN_IF_ERROR( invalid, "Ordered option given after named\n" ) - x264_free_string_array( arg ); + insert_opt( opt_str, length ); } - x264_free_string_array( split ); + + assert( offset == size ); return opts; } char *x264_get_option( const char *name, char **split_options ) { - if( !split_options ) - return NULL; - int last_i = -1; - for( int i = 0; split_options[i] != NULL; i += 2 ) - if( !strcmp( split_options[i], name ) ) - last_i = i; - if( last_i >= 0 ) - return split_options[last_i+1][0] ? split_options[last_i+1] : NULL; + if( split_options ) + { + int last_i = -1; + for( int i = 0; split_options[i]; i += 2 ) + if( !strcmp( split_options[i], name ) ) + last_i = i; + if( last_i >= 0 && split_options[last_i+1][0] ) + return split_options[last_i+1]; + } return NULL; } -int x264_otob( char *str, int def ) +int x264_otob( const char *str, int def ) { - int ret = def; if( str ) - ret = !strcasecmp( str, "true" ) || - !strcmp( str, "1" ) || - !strcasecmp( str, "yes" ); - return ret; + return !strcasecmp( str, "true" ) || !strcmp( str, "1" ) || !strcasecmp( str, "yes" ); + return def; } -double x264_otof( char *str, double def ) +double x264_otof( const char *str, double def ) { double ret = def; if( str ) @@ -192,7 +129,7 @@ double x264_otof( char *str, double def ) return ret; } -int x264_otoi( char *str, int def ) +int x264_otoi( const char *str, int def ) { int ret = def; if( str ) diff --git a/filters/filters.h b/filters/filters.h index 9bd12144..feceff4e 100644 --- a/filters/filters.h +++ b/filters/filters.h @@ -30,14 +30,11 @@ #include "x264cli.h" #include "filters/video/video.h" -char **x264_split_string( char *string, char *sep, int limit ); -void x264_free_string_array( char **array ); - -char **x264_split_options( const char *opt_str, const char *options[] ); +char **x264_split_options( const char *opt_str, const char * const *options ); char *x264_get_option( const char *name, char **split_options ); -int x264_otob( char *str, int def ); // option to bool -double x264_otof( char *str, double def ); // option to float/double -int x264_otoi( char *str, int def ); // option to int -char *x264_otos( char *str, char *def ); // option to string +int x264_otob( const char *str, int def ); // option to bool +double x264_otof( const char *str, double def ); // option to float/double +int x264_otoi( const char *str, int def ); // option to int +char *x264_otos( char *str, char *def ); // option to string #endif diff --git a/filters/video/crop.c b/filters/video/crop.c index bff6af0a..280347da 100644 --- a/filters/video/crop.c +++ b/filters/video/crop.c @@ -47,6 +47,20 @@ static void help( int longhelp ) printf( " removes pixels from the edges of the frame\n" ); } +static int handle_opts( crop_hnd_t *h, video_info_t *info, char **opts, const char * const *optlist ) +{ + for( int i = 0; i < 4; i++ ) + { + char *opt = x264_get_option( optlist[i], opts ); + FAIL_IF_ERROR( !opt, "%s crop value not specified\n", optlist[i] ) + h->dims[i] = x264_otoi( opt, -1 ); + FAIL_IF_ERROR( h->dims[i] < 0, "%s crop value `%s' is less than 0\n", optlist[i], opt ) + int dim_mod = i&1 ? (h->csp->mod_height << info->interlaced) : h->csp->mod_width; + FAIL_IF_ERROR( h->dims[i] % dim_mod, "%s crop value `%s' is not a multiple of %d\n", optlist[i], opt, dim_mod ) + } + return 0; +} + static int init( hnd_t *handle, cli_vid_filter_t *filter, video_info_t *info, x264_param_t *param, char *opt_string ) { FAIL_IF_ERROR( x264_cli_csp_is_invalid( info->csp ), "invalid csp %d\n", info->csp ) @@ -55,21 +69,16 @@ static int init( hnd_t *handle, cli_vid_filter_t *filter, video_info_t *info, x2 return -1; h->csp = x264_cli_get_csp( info->csp ); - static const char *optlist[] = { "left", "top", "right", "bottom", NULL }; + static const char * const optlist[] = { "left", "top", "right", "bottom", NULL }; char **opts = x264_split_options( opt_string, optlist ); if( !opts ) return -1; - for( int i = 0; i < 4; i++ ) - { - char *opt = x264_get_option( optlist[i], opts ); - FAIL_IF_ERROR( !opt, "%s crop value not specified\n", optlist[i] ) - h->dims[i] = x264_otoi( opt, -1 ); - FAIL_IF_ERROR( h->dims[i] < 0, "%s crop value `%s' is less than 0\n", optlist[i], opt ) - int dim_mod = i&1 ? (h->csp->mod_height << info->interlaced) : h->csp->mod_width; - FAIL_IF_ERROR( h->dims[i] % dim_mod, "%s crop value `%s' is not a multiple of %d\n", optlist[i], opt, dim_mod ) - } - x264_free_string_array( opts ); + int err = handle_opts( h, info, opts, optlist ); + free( opts ); + if( err ) + return -1; + h->dims[2] = info->width - h->dims[0] - h->dims[2]; h->dims[3] = info->height - h->dims[1] - h->dims[3]; FAIL_IF_ERROR( h->dims[2] <= 0 || h->dims[3] <= 0, "invalid output resolution %dx%d\n", h->dims[2], h->dims[3] ) diff --git a/filters/video/depth.c b/filters/video/depth.c index aa59d7e9..000ec1ef 100644 --- a/filters/video/depth.c +++ b/filters/video/depth.c @@ -200,7 +200,7 @@ static int init( hnd_t *handle, cli_vid_filter_t *filter, video_info_t *info, if( opt_string ) { - static const char *optlist[] = { "bit_depth", NULL }; + static const char * const optlist[] = { "bit_depth", NULL }; char **opts = x264_split_options( opt_string, optlist ); if( opts ) @@ -211,7 +211,7 @@ static int init( hnd_t *handle, cli_vid_filter_t *filter, video_info_t *info, ret = bit_depth < 8 || bit_depth > 16; csp = bit_depth > 8 ? csp | X264_CSP_HIGH_DEPTH : csp & ~X264_CSP_HIGH_DEPTH; change_fmt = (info->csp ^ csp) & X264_CSP_HIGH_DEPTH; - x264_free_string_array( opts ); + free( opts ); } else ret = 1; diff --git a/filters/video/resize.c b/filters/video/resize.c index 5ccbffb9..1c061ea4 100644 --- a/filters/video/resize.c +++ b/filters/video/resize.c @@ -214,7 +214,7 @@ static int pick_closest_supported_csp( int csp ) return ret; } -static int handle_opts( const char **optlist, char **opts, video_info_t *info, resizer_hnd_t *h ) +static int handle_opts( const char * const *optlist, char **opts, video_info_t *info, resizer_hnd_t *h ) { uint32_t out_sar_w, out_sar_h; @@ -416,7 +416,7 @@ static int init( hnd_t *handle, cli_vid_filter_t *filter, video_info_t *info, x2 if( !opt_string && !full_check( info, param ) ) return 0; - static const char *optlist[] = { "width", "height", "sar", "fittobox", "csp", "method", NULL }; + static const char * const optlist[] = { "width", "height", "sar", "fittobox", "csp", "method", NULL }; char **opts = x264_split_options( opt_string, optlist ); if( !opts && opt_string ) return -1; @@ -424,6 +424,9 @@ static int init( hnd_t *handle, cli_vid_filter_t *filter, video_info_t *info, x2 resizer_hnd_t *h = calloc( 1, sizeof(resizer_hnd_t) ); if( !h ) return -1; + + h->ctx_flags = convert_method_to_flag( x264_otos( x264_get_option( optlist[5], opts ), "" ) ); + if( opts ) { h->dst_csp = info->csp; @@ -432,14 +435,20 @@ static int init( hnd_t *handle, cli_vid_filter_t *filter, video_info_t *info, x2 h->dst.range = info->fullrange; // maintain input range if( !strcmp( opt_string, "normcsp" ) ) { + free( opts ); /* only in normalization scenarios is the input capable of changing properties */ h->variable_input = 1; h->dst_csp = pick_closest_supported_csp( info->csp ); FAIL_IF_ERROR( h->dst_csp == X264_CSP_NONE, "filter get invalid input pixel format %d (colorspace %d)\n", convert_csp_to_pix_fmt( info->csp ), info->csp ) } - else if( handle_opts( optlist, opts, info, h ) ) - return -1; + else + { + int err = handle_opts( optlist, opts, info, h ); + free( opts ); + if( err ) + return -1; + } } else { @@ -448,8 +457,6 @@ static int init( hnd_t *handle, cli_vid_filter_t *filter, video_info_t *info, x2 h->dst.height = param->i_height; h->dst.range = param->vui.b_fullrange; // change to libx264's range } - h->ctx_flags = convert_method_to_flag( x264_otos( x264_get_option( optlist[5], opts ), "" ) ); - x264_free_string_array( opts ); if( h->ctx_flags != SWS_FAST_BILINEAR ) h->ctx_flags |= SWS_FULL_CHR_H_INT | SWS_FULL_CHR_H_INP | SWS_ACCURATE_RND;