[FFmpeg-devel] [patch] glob matching for image series

Nicolas George nicolas.george at normalesup.org
Sun Jan 29 19:37:36 CET 2012


Le duodi 2 pluviôse, an CCXX, Brian Olson a écrit :
> > Simplified is_glob() using strcspn() (cool, hadn't seen that part of the
> > C strings standard library before, thanks Nicolas)
> > 
> > Fixed usage of first_index and last_index so that they shouldn't be used
> > uninitted.

Sorry to have kept you waiting, I was otherwise busy.

> 
> diff --git a/configure b/configure
> index 3bace2b..474ef93 100755
> --- a/configure
> +++ b/configure
> @@ -1179,6 +1179,7 @@ HAVE_LIST="
>      GetProcessMemoryInfo
>      GetProcessTimes
>      getrusage
> +    glob
>      gnu_as
>      ibm_asm
>      inet_aton
> @@ -3021,6 +3022,7 @@ check_func_headers windows.h GetProcessAffinityMask
>  check_func_headers windows.h GetProcessTimes
>  check_func_headers windows.h MapViewOfFile
>  check_func_headers windows.h VirtualAlloc
> +check_func_headers glob.h glob
>  
>  check_header dlfcn.h
>  check_header dxva2api.h -D_WIN32_WINNT=0x0600
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index e2f6add..690bd83 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -1226,7 +1226,11 @@ ffmpeg -f image2 -i foo-%03d.jpeg -r 12 -s WxH foo.avi
>  The syntax @code{foo-%03d.jpeg} specifies to use a decimal number
>  composed of three digits padded with zeroes to express the sequence
>  number. It is the same syntax supported by the C printf function, but
> -only formats accepting a normal integer are suitable.
> +only formats accepting a normal integer are suitable. When importing
> +an image sequence, -i also accepts shell-like wildcard patterns such as
> + at code{foo-*.jpeg}, @code{foo-???.jpeg} or @code{foo-00[234]*.jpeg}.
> +It will probably be necessary to escape these patterns so they do not
> +get interpreted by your shell.
>  
>  @item
>  You can put many streams of the same type in the output:
> diff --git a/libavformat/img2.c b/libavformat/img2.c
> index bc35591..17eb899 100644
> --- a/libavformat/img2.c
> +++ b/libavformat/img2.c
> @@ -29,6 +29,25 @@
>  #include "avformat.h"
>  #include "avio_internal.h"
>  #include "internal.h"
> +#if HAVE_GLOB
> +#include <glob.h>
> +
> +/* Locally define as 0 (bitwise-OR no-op) any missing glob options that
> +   are non-posix glibc/bsd extensions. */
> +#ifndef GLOB_NOMAGIC
> +#define GLOB_NOMAGIC 0
> +#endif
> +#ifndef GLOB_TILDE
> +#define GLOB_TILDE 0
> +#endif
> +#ifndef GLOB_TILDE_CHECK
> +#define GLOB_TILDE_CHECK GLOB_TILDE
> +#endif
> +#ifndef GLOB_BRACE
> +#define GLOB_BRACE 0
> +#endif
> +
> +#endif /* HAVE_GLOB */
>  
>  typedef struct {
>      const AVClass *class;  /**< Class for private options. */
> @@ -44,6 +63,10 @@ typedef struct {
>      char *framerate;        /**< Set by a private option. */
>      int loop;
>      int updatefirst;
> +    int use_glob;
> +#if HAVE_GLOB
> +    glob_t globstate;
> +#endif
>  } VideoData;
>  
>  typedef struct {
> @@ -133,6 +156,25 @@ static enum CodecID av_str2id(const IdStrMap *tags, const char *str)
>      return CODEC_ID_NONE;
>  }
>  
> +#if HAVE_GLOB
> +static int glob_errfunc(const char *epath, int glob_error)
> +{
> +    return 0;
> +}

According to Single Unix, passing NULL as errfunc would have the same
effect. Do you have any reason not to use that feature?

> +
> +static int is_glob(const char *path)
> +{
> +    size_t span = strcspn(path, "*?[]{}\\");
> +    /* Did we hit a glob char or get to the end? */
> +    return path[span] != '\0';
> +}
> +#else
> +static int is_glob(const char *path)
> +{
> +    return 0;
> +}
> +#endif
> +
>  /* return -1 if no image found */
>  static int find_image_range(int *pfirst_index, int *plast_index,
>                              const char *path)
> @@ -192,6 +234,8 @@ static int read_probe(AVProbeData *p)
>      if (p->filename && av_str2id(img_tags, p->filename)) {
>          if (av_filename_number_test(p->filename))
>              return AVPROBE_SCORE_MAX;
> +        else if (is_glob(p->filename))
> +            return AVPROBE_SCORE_MAX;
>          else
>              return AVPROBE_SCORE_MAX/2;
>      }
> @@ -263,8 +307,23 @@ static int read_header(AVFormatContext *s1, AVFormatParameters *ap)
>      }
>  
>      if (!s->is_pipe) {
> -        if (find_image_range(&first_index, &last_index, s->path) < 0)
> -            return AVERROR(ENOENT);
> +        if (is_glob(s->path)) {
> +#if HAVE_GLOB
> +            int gerr;
> +            s->use_glob = 1;
> +            gerr = glob(s->path, GLOB_NOCHECK|GLOB_BRACE|GLOB_NOMAGIC|GLOB_TILDE_CHECK, glob_errfunc, &s->globstate);
> +            if (gerr != 0) {
> +                return AVERROR(ENOENT);
> +            }
> +            first_index = 0;
> +            last_index = s->globstate.gl_pathc - 1;
> +#endif
> +        } else {
> +            s->use_glob = 0;
> +            if (find_image_range(&first_index, &last_index, s->path) < 0) {
> +                return AVERROR(ENOENT);
> +            }

Unless I am mistaken, these lines are only reindented and get a useless pair
of braces: could you restore them to their original state, to make the diff
easier to follow?

If I end up applying the patch, I would commit the reindent afterwards.

> +        }
>          s->img_first = first_index;
>          s->img_last = last_index;
>          s->img_number = first_index;
> @@ -296,7 +355,8 @@ static int read_header(AVFormatContext *s1, AVFormatParameters *ap)
>  static int read_packet(AVFormatContext *s1, AVPacket *pkt)
>  {
>      VideoData *s = s1->priv_data;
> -    char filename[1024];
> +    char filename_bytes[1024];
> +    char *filename = filename_bytes;
>      int i;
>      int size[3]={0}, ret[3]={0};
>      AVIOContext *f[3];
> @@ -309,9 +369,17 @@ static int read_packet(AVFormatContext *s1, AVPacket *pkt)
>          }
>          if (s->img_number > s->img_last)
>              return AVERROR_EOF;
> -        if (av_get_frame_filename(filename, sizeof(filename),
> -                                  s->path, s->img_number)<0 && s->img_number > 1)
> -            return AVERROR(EIO);
> +        if (s->use_glob) {
> +#if HAVE_GLOB
> +            filename = s->globstate.gl_pathv[s->img_number];
> +#endif
> +        } else {
> +            if (av_get_frame_filename(
> +                    filename_bytes, sizeof(filename_bytes),
> +                    s->path, s->img_number)<0 &&
> +                s->img_number > 1)
> +                return AVERROR(EIO);

Same thing here: I think this line is unchanged, and I would like to be
sure.

> +        }
>          for(i=0; i<3; i++){
>              if (avio_open2(&f[i], filename, AVIO_FLAG_READ,
>                             &s1->interrupt_callback, NULL) < 0) {
> @@ -361,6 +429,16 @@ static int read_packet(AVFormatContext *s1, AVPacket *pkt)
>      }
>  }
>  
> +static int read_close(struct AVFormatContext* s1) {
> +    VideoData *s = s1->priv_data;
> +#if HAVE_GLOB
> +    if (s->use_glob) {
> +        globfree(&s->globstate);
> +    }
> +#endif
> +    return 0;
> +}
> +
>  #if CONFIG_IMAGE2_MUXER || CONFIG_IMAGE2PIPE_MUXER
>  /******************************************************/
>  /* image output */
> @@ -494,6 +572,7 @@ AVInputFormat ff_image2_demuxer = {
>      .read_probe     = read_probe,
>      .read_header    = read_header,
>      .read_packet    = read_packet,
> +    .read_close     = read_close,
>      .flags          = AVFMT_NOFILE,
>      .priv_class     = &img2_class,
>  };
> 
> 

On the whole, it looks quite good. Does anyone object to this?

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120129/39f87315/attachment.asc>


More information about the ffmpeg-devel mailing list