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

[Michael Niedermayer]

On Sun, Mar 11, 2012 at 05:46:08PM +0100, Alexander Strasser wrote:
> Hi Nicolas,
> 
>   I assume you read my reply to Brian in the meanwhile, therefore
> some relevant parts of this mail will be left uncommented.
> 
> Nicolas George wrote:
> > Le nonidi 9 ventôse, an CCXX, Alexander Strasser a écrit :
> > >   1. Support globbing not only for picture input.
> > 
> > How do you imagine it should be handled, from the API point of view? We are
> > nowhere near supporting demuxer-level concatenation, IMHO, so the syntax for
> > various parts of the command line will have to change four or five times
> > before we are able of implementing anything like that.
> > 
> > >   2. Do it only if it was explicitly enabled
> > 
> > I find that rather user-unfriendly. OTOH, having options to ensure the mode
> > of expansion would be useful for robust automated scripts:
> 
>   I see. What do you think about the attached patch? It builds directly on
> Brian's work and tries to be user-friendly and consistent while not making
> currently working use cases more hard/impossible when globbing is compiled
> in.
> 
>   The few examples are given in the patch commit message. I also attached
> my complete test log so you can dig in deeper. Beware the examples found
> in the test log are rather extreme to test more corner cases.
> 
> > ffmpeg -f img2 -expand printf photo%05d.jpg
> > ffmpeg -f img2 -expand glob   photo\*.jpg
> 
>   Looks like a nice idea. I am OK with adding such features later on.
> 
> > >   3. Agree on a set of features like ?*[]{} and fail if is not available
> > >      (maybe with the additional possibility to force it on with what is
> > >       available)
> > 
> > I do not like it at all in principle: if we were to cripple features just to
> > be compatible with the lowest common denominator, we could just all install
> > windows and be done with it.
> > 
> > On the other hand, for this particular case:
> > 
> > - GLOB_BRACE, could be marginally useful, but also cause trouble (the files
> >   that randomly appears on people's hard drives tend to have braces or
> >   parentheses to indicate the checksum, name of the team or whatever, or so
> >   I have been told).
> 
>   This tends to be no problem with my proposed patch. Very useful glob
> chars, [], are also used frequently for the same purposes you quote above.
> This works out too with my patch.
> 
> > - GLOB_TILDE(_CHECK): this is not related to globbing at all, this is some
> >   kind of special syntax for a bizarre environment variable expansion (~
> >   expands like $HOME, but only at the start; ~foo would expand like
> >   $HOME_FOO, assuming the contents of passwd is part of the environment).
> >   Also, while it is perfectly normal for 'foo*.jpg' to be supported only by
> >   the img2 demuxer, there is no logic of being able to write ~/image.jpg and
> >   not ~/video.avi.
> > 
> >   Also, for the command-line tool, the user can just not escape the ~ and
> >   let their shell expand it.
> 
>   So you think we can just drop it?
> 
> > - GLOB_NOMAGIC should be useless, since glob is called only if is_glob has
> >   already detected metacharacters.
> 
>   Agreed. Though didn't change it in this patch though.
> 
> > So in this particular case, I do not mind sticking to the standard features
> > at all.
> 
>   Ok, tell me if I should prepare a patch removing the GLOB_BRACE,
> GLOB_TILDE(_CHECK), GLOB_NOMAGIC.
>  
> > >   4. Tell the user if he requested to enable globbing but it wasn't
> > >      available at all in his lavf binary.
> > 
> > I second that.
> 
>   I wanted to do this as an incremental improvement to not further
> delay Brian's work getting into ffmpeg.
> 
>   I thought about printing a warning if the user used glob chars in
> image2 input path but glob support was not compiled in. Did you have
> something else in mind?
> 
>   If we decide to keep GLOB_BRACE, that should be taken into account too.
> 
>   Also I think these warnings will have much lower chance of unexpected
> occurrence after my patch, because only enabled glob chars would count
> (e.g. %[).
> 
>   BTW if my patch passes review I can of course write an update for the
> documentation and resubmit.
> 
>   While writing this a further question pops up, should ] and } be
> removed in is_glob() detection code? If don't think it will make much
> sense to try to glob it if we only found closing brackets/braces.
> 
> > >   The described behaviour would allow for implementation of applications
> > > that behave consistently across platforms. That is a property I appreciate
> > > very much and that I would like to see from FFmpeg programs and libraries.
> > 
> > Unfortunately, consistency is often at odds with feature-fullness, and
> > someone who decided to use one particular platform because it has some
> > interesting extensions should be able to benefit from these extensions.
> > 
> > What can be done, OTOH, is to make sure that someone who wants to stick to
> > the common denominator can avoid triggering unexpected extensions.
> 
>   Not sure I can fully agree. It is very broad statement and therefore
> not easy to comment on specifically. But in any case I think we should
> get along with each other :)
> 
>   Alexander

>  img2dec.c |   30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 0b95708302972c93e8055e4b040b2a3451d2bd50  0001-img2dec-Recognize-glob-meta-chars-only-if-prefixed-b.patch
> From e2e044ff23cb28b6c885d041cdd3a59f47c3ec4a Mon Sep 17 00:00:00 2001
> From: Alexander Strasser <eclipse7 at gmx.net>
> Date: Sun, 11 Mar 2012 15:24:51 +0100
> Subject: [PATCH] img2dec: Recognize glob meta chars only if prefixed by %
> 
>   This changes globbing support to only be used if the character
> contains at least one glob meta character that is preceded by
> an unescaped %. To escape a literal % one would use %% which is
> identical to the way to match a % with image2 sequence generation
> feature.
> 
> * Makes it possible to have patterns like %04d-[720p].jpg work
>   again with sequence number generation. Previously this would
>   always be detected as a glob pattern and was interpreted by
>   the image2 glob code instead.
> 
> * Makes it possible to use %*-[720p].jpg to match above pattern
>   without having to double escape it to be not interpreted by most
>   shells and not by the image2 glob code (previously one would
>   need to use \*-\\\[720p\\\].jpg to achieve the same)

applied

thanks!

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- 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/20120330/11ef3d2f/attachment.asc>