[FFmpeg-devel] [patch] allow wordexp globs in image2 file sequence import

[Aurelien Jacobs]

On Sat, Jan 01, 2011 at 04:27:22PM -0500, Brian Olson wrote:
> 
> > On Fri, Dec 31, 2010 at 18:11, Brian Olson wrote:
> >> It seemed cumbersome to me to have to symlink/rename my image sequence files to fit
> >> ffmpeg -i prefix%05d.jpg
> >> so I hacked libavformat/img2.c to use <wordexp.h> file globbing, and now I can
> >> ffmpeg -i '*.jpg'
> >> (and other shell-expnsion patterns like ? * [] {})
> >> 
> >> This is my first attempt at submitting to ffmpeg. I hope you like it. Feedback welcome. Hopefully I didn't break the conventions too much.
> > 
> > I think you should check for wordexp() availability in configure, and
> > guard your code with HAVE_WORDEXP.
> > grep the code for strtok_r/HAVE_STRTOK_R, as an example.
> > 
> > Also, you should update the documentation.
> 
> Here's an expanded patch with documentation, a change to configure (and I'd like to note that this configure script is a lot nicer than autoconf), and #if HAVE_WORDEXP inserted around my work in libavformat/img2.c
> 
> Carl Eugen had a comment about indention and line wraps. I wrapped one long line to <80 chars. I found in doc/developer the guideline that long blocks (>5 lines) included in new if(){} blocks should have a separate indention-fixup patch. I have two changes right on the limit of those 5 lines, and I'd hope it's ultimately simpler to commit this all as one change.
> 
> [...]
>  
> +#if HAVE_WORDEXP
> +static int is_wordexp(const char* path) {
> +    while (*path != '\0') {
> +        switch (*path) {
> +        case '*':
> +        case '?':
> +        case '[':
> +        case ']':
> +        case '{':
> +        case '}':
> +        case '\\':
> +            return 1;
> +        }
> +        path++;
> +    }
> +    return 0;
> +}

This function can be reduced to:

static int is_wordexp(const char* path) {
    return strcspn(path,"*?[]{}\\") != strlen(path);
}

> +static int use_wordexp(VideoData *s) {
> +    return s->use_wordexp;
> +}

This function is just a useless and ugly indirection.
Drop it and access s->use_wordexp directly instead (get it out of
the ifdef).

Aurel