cli: Refactor filter option parsing
authorHenrik Gramner <henrik@gramner.com>
Thu, 4 Feb 2016 19:06:57 +0000 (20:06 +0100)
committerHenrik Gramner <henrik@gramner.com>
Tue, 12 Apr 2016 15:10:29 +0000 (17:10 +0200)
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.

filters/filters.c
filters/filters.h
filters/video/crop.c
filters/video/depth.c
filters/video/resize.c

index aecd296..204dda5 100644 (file)
@@ -5,6 +5,7 @@
  *
  * Authors: Diogo Franco <diogomfranco@gmail.com>
  *          Steven Walters <kemuri9@gmail.com>
+ *          Henrik Gramner <henrik@gramner.com>
  *
  * 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
 #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 )
index 9bd1214..feceff4 100644 (file)
 #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
index bff6af0..280347d 100644 (file)
@@ -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] )
index aa59d7e..000ec1e 100644 (file)
@@ -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;
index 5ccbffb..1c061ea 100644 (file)
@@ -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;