[FFmpeg-devel] [PATCH] lavf/img2dec: add -pattern_type option

[Stefano Sabatini]

On date Tuesday 2012-08-07 01:43:44 +0200, Alexander Strasser encoded:
> Hi Stefano,
> 
>   thanks for tackling this!
> 
>   On removal of glob_sequence pattern we can also remove
> is_glob function (and therefore adjust read_probe). Just
> writing it here as a reminder that we not forget about that.
> 
> Stefano Sabatini wrote:
> > Allow to override the default 'glob_sequence' value, which is deprecated
> > in favor of the new 'glob' and 'sequence' options.
> > 
> > The new pattern types should be easier on the user since they are more
> > predictable than 'glob_sequence', and do not require awkward escaping.
> > 
> > FIXME: bump micro before pushing
> > ---
> >  libavformat/img2dec.c |   34 +++++++++++++++++++++++++++++++++-
> >  1 files changed, 33 insertions(+), 1 deletions(-)
> > 
> > diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
> > index 0443b1a..a108656 100644
> > --- a/libavformat/img2dec.c
> > +++ b/libavformat/img2dec.c
> > @@ -41,6 +41,8 @@
> >  
> >  #endif /* HAVE_GLOB */
> >  
> > +enum PatternType { PT_GLOB_SEQUENCE, PT_GLOB, PT_SEQUENCE };
> > +
> >  typedef struct {
> >      const AVClass *class;  /**< Class for private options. */
> >      int img_first;
> > @@ -54,6 +56,7 @@ typedef struct {
> >      char *video_size;       /**< Set by a private option. */
> >      char *framerate;        /**< Set by a private option. */
> >      int loop;
> > +    enum PatternType pattern_type;
> >      int use_glob;
> >  #if HAVE_GLOB
> >      glob_t globstate;
> > @@ -233,6 +236,9 @@ static int read_header(AVFormatContext *s1)
> >      }
> >  
> >      if (!s->is_pipe) {
> > +        if (s->pattern_type == PT_GLOB_SEQUENCE) {
> > +            av_log(s1, AV_LOG_WARNING, "Pattern type 'glob_sequence' is deprecated: "
> > +                   "use -pattern_type 'glob' or 'sequence' instead\n");
> >          s->use_glob = is_glob(s->path);
> >          if (s->use_glob) {
> 
>   I would prefer to only print the warning if is_glob() is true to
> not spam users not aware of the globbing functionality.
> 
>   Also a more ffmpeg tool agnostic message like this could be used:
> 
> "Using deprecated pattern_type 'glob_sequence' for globbing: port your pattern to 'glob' because 'glob_sequence' will be removed\n"
> 
>   If you want to keep the hint on how it is exposed as ffmpeg
> option you could still keep the spirit and reduce to say that the
> user needs to adapt to "-pattern_type glob" as it is unlikely the
> user wanted to use sequence if we got into "if (s->use_glob)" branch.

Done this way (note that this also has its drawbacks, e.g. in the case
the user choose to explicitly use glob_sequence).

> 
> >  #if HAVE_GLOB
> > @@ -260,7 +266,9 @@ static int read_header(AVFormatContext *s1)
> >              first_index = 0;
> >              last_index = s->globstate.gl_pathc - 1;
> >  #endif
> > -        } else {
> > +        }
> > +        }
> > +        if ((s->pattern_type == PT_GLOB_SEQUENCE && !s->use_glob) || s->pattern_type == PT_SEQUENCE) {
> >              if (find_image_range(&first_index, &last_index, s->path,
> >                                   s->start_number, s->start_number_range) < 0) {
> >                  av_log(s1, AV_LOG_ERROR,
> > @@ -268,7 +276,25 @@ static int read_header(AVFormatContext *s1)
> >                         s->path, s->start_number, s->start_number + s->start_number_range - 1);
> >                  return AVERROR(ENOENT);
> >              }
> > +        } else if (s->pattern_type == PT_GLOB) {
> > +            int gerr;
> > +            if (!HAVE_GLOB) {
> > +                av_log(s1, AV_LOG_ERROR, "Pattern type 'glob' was selected but is not supported by libavformat\n");
> 
> nit: "Pattern type 'glob' was selected but is not supported by this libavformat build\n");
> or   "is not available in this libavformat build"
> or similar.
> 
>   Leave it as is or change to your liking.
> 
> [...]
> 
>   Besides the missing #ifdef Nicolas mentioned, the patch looks fine to me.
> 
>   I also like this solution better then the current solution. A plus are the
> clear failure modes.

Updated.
-- 
FFmpeg = Freak and Forgiving Mythic Purposeless Evil Geisha
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-lavf-img2dec-add-and-document-pattern_type-option.patch
Type: text/x-diff
Size: 9672 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120807/c04ff1d5/attachment.bin>