Re: [PATCH 2/4] libmpdemux/mf: Refactor into one function per pattern type
Alexander Strasser <[email protected]> Wed, 22 Sep 2021 21:54:08 +0200
| Newsgroups | gmane.comp.video.mplayer.devel |
|---|---|
| Message-ID | <[email protected]> |
On 2021-06-01 22:05 +0200, Alexander Strasser wrote:
[...]
> @@ -158,14 +158,45 @@ mf_t* open_mf(char * filename){
> // mp_msg( MSGT_STREAM,MSGL_V,"[mf] added file %d.: %s\n",mf->nr_of_files,mf->names[mf->nr_of_files] );
> mf->nr_of_files++;
> }
> -
> - fname = NULL;
> }
>
> mp_msg( MSGT_STREAM,MSGL_INFO,"[mf] number of files: %d\n",mf->nr_of_files );
> + return 1;
> +}
> +
> +
> +mf_t* open_mf(char * filename){
> +#if defined(HAVE_GLOB) || defined(__MINGW32__)
> + mf_t * mf;
> + int init_success = 0;
> +
> + mf=calloc( 1,sizeof( mf_t ) );
> +
> + if( filename[0] == '@' )
> + {
> + init_success = init_mf_from_list_file(mf, filename);
> + }
> + /* only try one more init method depending on filename */
Despite this comment the following wasn't really understand:
> + if( !init_success && strchr( filename,',') )
> + {
> + init_success = init_mf_from_comma_delimited_paths(mf, filename);
> + }
> + else if ( !init_success && !strchr( filename,'%' ) )
> + {
> + init_success = init_mf_from_glob_pattern(mf, filename);
> + }
> + else if ( !init_success )
> + {
> + init_success = init_mf_from_printf_format(mf, filename);
> + }
I applied this fixup locally to make it more readable (amended
patch attached):
--->8---
--- a/libmpdemux/mf.c
+++ b/libmpdemux/mf.c
@@ -197,19 +197,21 @@ mf_t* open_mf(char * filename){
init_success = init_mf_from_list_file(mf, filename);
}
- /* only try one more init method depending on filename */
- if( !init_success && strchr( filename,',') )
+ if( !init_success )
+ {
+ if( strchr( filename,',') )
{
init_success = init_mf_from_comma_delimited_paths(mf, filename);
}
- else if ( !init_success && !strchr( filename,'%' ) )
+ else if ( !strchr( filename,'%' ) )
{
init_success = init_mf_from_glob_pattern(mf, filename);
}
- else if ( !init_success )
+ else
{
init_success = init_mf_from_printf_format(mf, filename);
}
+ }
if (!init_success)
{
---8<---
I want to apply the whole patch set without much of a warning soon-ish.
If there are any more comments, now would be a good time.
Thanks,
Alexander
> +
> + if (!init_success)
> + {
> + free(mf);
> + return NULL;
> + }
>
> -exit_mf:
> - free( fname );
> return mf;
> #else
> mp_msg(MSGT_STREAM,MSGL_FATAL,"[mf] mf support is disabled on your os\n");
_______________________________________________
MPlayer-dev-eng mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
0002-libmpdemux-mf-Refactor-into-one-function-per-pattern.patch
(text/x-diff, 4.8 KB)
From 11651421ef1e5083f3080194780d9883076c6192 Mon Sep 17 00:00:00 2001 From: Alexander Strasser <[email protected]> Date: Fri, 30 Apr 2021 11:05:21 +0200 Subject: [PATCH] libmpdemux/mf: Refactor into one function per pattern type Make the code more readable; especially regarding the usage of the now often shared local variables. --- libmpdemux/mf.c | 95 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 64 insertions(+), 31 deletions(-) diff --git a/libmpdemux/mf.c b/libmpdemux/mf.c index d4f5be98b..99ae3053f 100644 --- a/libmpdemux/mf.c +++ b/libmpdemux/mf.c @@ -46,26 +46,15 @@ int mf_h = 0; //288; double mf_fps = 25.0; char * mf_type = NULL; //"jpg"; -mf_t* open_mf(char * filename){ -#if defined(HAVE_GLOB) || defined(__MINGW32__) - glob_t gg; - struct stat fs; - int i; - char * fname; - mf_t * mf; - int error_count = 0; - int count = 0; - mf=calloc( 1,sizeof( mf_t ) ); - - if( filename[0] == '@' ) - { +static int init_mf_from_list_file(mf_t* mf, const char * filename){ FILE *lst_f=fopen(filename + 1,"r"); if ( lst_f ) { - fname=malloc(PATH_MAX); + char *fname=malloc(PATH_MAX); while ( fgets( fname,PATH_MAX,lst_f ) ) { + struct stat fs; /* remove spaces from end of fname */ char *t=fname + strlen( fname ) - 1; while ( t > fname && isspace( *t ) ) *(t--)=0; @@ -83,17 +72,21 @@ mf_t* open_mf(char * filename){ fclose( lst_f ); mp_msg( MSGT_STREAM,MSGL_INFO,"[mf] number of files: %d\n",mf->nr_of_files ); - goto exit_mf; + free( fname ); + return 1; } mp_msg( MSGT_STREAM,MSGL_INFO,"[mf] %s is not indirect filelist\n",filename+1 ); - } + return 0; +} - if( strchr( filename,',') ) - { + +static int init_mf_from_comma_delimited_paths(mf_t* mf, char * filename){ + char * fname; mp_msg( MSGT_STREAM,MSGL_INFO,"[mf] filelist: %s\n",filename ); while ( ( fname=strsep( &filename,"," ) ) ) { + struct stat fs; if ( stat( fname,&fs ) ) { mp_msg( MSGT_STREAM,MSGL_V,"[mf] file not found: '%s'\n",fname ); @@ -107,13 +100,14 @@ mf_t* open_mf(char * filename){ } } mp_msg( MSGT_STREAM,MSGL_INFO,"[mf] number of files: %d\n",mf->nr_of_files ); + return 1; +} - goto exit_mf; - } - if ( !strchr( filename,'%' ) ) - { - fname=malloc( strlen( filename ) + 32 ); +static int init_mf_from_glob_pattern(mf_t* mf, const char * filename){ + glob_t gg; + char *fname=malloc( strlen( filename ) + 32 ); + int i; strcpy( fname,filename ); if ( !strchr( filename,'*' ) ) strcat( fname,"*" ); @@ -121,7 +115,7 @@ mf_t* open_mf(char * filename){ mp_msg( MSGT_STREAM,MSGL_INFO,"[mf] search expr: %s\n",fname ); if ( glob( fname,0,NULL,&gg ) ) - { free( mf ); free( fname ); return NULL; } + { free( fname ); return 0; } mf->nr_of_files=gg.gl_pathc; mf->names=calloc( gg.gl_pathc, sizeof( char* ) ); @@ -130,20 +124,26 @@ mf_t* open_mf(char * filename){ for( i=0;i < gg.gl_pathc;i++ ) { + struct stat fs; if (stat( gg.gl_pathv[i],&fs ) == -1) continue; if( S_ISDIR( fs.st_mode ) ) continue; mf->names[i]=strdup( gg.gl_pathv[i] ); // mp_msg( MSGT_STREAM,MSGL_DBG2,"[mf] added file %d.: %s\n",i,mf->names[i] ); } + free( fname ); globfree( &gg ); - goto exit_mf; - } + return 1; +} + +static int init_mf_from_printf_format(mf_t* mf, const char * filename){ + int count = 0, error_count = 0; mp_msg( MSGT_STREAM,MSGL_INFO,"[mf] search expr: %s\n",filename ); while ( error_count < 5 ) { - fname = mp_asprintf( filename,count++ ); + struct stat fs; + char *fname = mp_asprintf( filename,count++ ); if ( stat( fname,&fs ) ) { @@ -158,14 +158,47 @@ mf_t* open_mf(char * filename){ // mp_msg( MSGT_STREAM,MSGL_V,"[mf] added file %d.: %s\n",mf->nr_of_files,mf->names[mf->nr_of_files] ); mf->nr_of_files++; } - - fname = NULL; } mp_msg( MSGT_STREAM,MSGL_INFO,"[mf] number of files: %d\n",mf->nr_of_files ); + return 1; +} + + +mf_t* open_mf(char * filename){ +#if defined(HAVE_GLOB) || defined(__MINGW32__) + mf_t * mf; + int init_success = 0; + + mf=calloc( 1,sizeof( mf_t ) ); + + if( filename[0] == '@' ) + { + init_success = init_mf_from_list_file(mf, filename); + } + + if( !init_success ) + { + if( strchr( filename,',') ) + { + init_success = init_mf_from_comma_delimited_paths(mf, filename); + } + else if ( !strchr( filename,'%' ) ) + { + init_success = init_mf_from_glob_pattern(mf, filename); + } + else + { + init_success = init_mf_from_printf_format(mf, filename); + } + } + + if (!init_success) + { + free(mf); + return NULL; + } -exit_mf: - free( fname ); return mf; #else mp_msg(MSGT_STREAM,MSGL_FATAL,"[mf] mf support is disabled on your os\n"); --