[Ffmpeg-devel] [PATCH] Wildcard and catalog image sequences

[Michael Niedermayer]

Hi

On Wed, Aug 30, 2006 at 02:12:01PM +0200, Michel Bardiaux wrote:
> Michael Niedermayer wrote:
> >Hi
> >
> >On Wed, Aug 30, 2006 at 11:21:53AM +0200, Michel Bardiaux wrote:
> >>Michael Niedermayer wrote:
> >>>Hi
> >>>
> >>>On Tue, Aug 29, 2006 at 04:18:35PM +0200, Michel Bardiaux wrote:
> >>>>First attempt, awaiting comments.
> >>>>
> >>>>Basic idea: -fimage2 supporting 2 new kinds of (input) sequences:
> >>>>
> >>>>-f image2 'dir/x*.jpg' Guess what?
> >>>>
> >>>>-f image2 @some_url where some_url designates a text file (or whatever)
> >>>>containing the url of each image, 1 per line.
> >>>>
> >>>>There are a few pending issues:
> >>>>
> >>>>1. In img.c (-fimage) I support only numbered. It supports only
> >>>>  GIF now and will be deprecated. IMHO no debate.
> >>>>
> >>>>2. Numbered sequences only for output. IMHO no debate.
> >>>>
> >>>>3. Wildcard patterns use only * and ? (no [] or {} as in most
> >>>>  UNIX shells). Assume /-separated path components.
> >>>>  Case-sensitive. Sorted case-sensitive. Using opendir.
> >>>>  The latter is autoconfigured, but I have no idea how to correctly 
> >>>>support
> >>>>  case-insensitivity and / in filenames, since cross-compilation has to
> >>>>  be supported.
> >>>>
> >>>>4. In @-sequences, I trim any \r or space at end of line. Debatable.
> >>>>
> >>>>5. No doc patch yet.
> >>>somehow i feel that it would be better to leave the wildcard handling to 
> >>>the
> >>>shell, maybe something with a syntax looking like:
> >>>ffmpeg -i `echo *.jpg | tr ' ' '*'` ...
> >>>that way, image lists could also trivially be support by
> >>>ffmpeg -i `cat filelist | tr ' ' '*'` ...
> >>>and numbered stuff can be supported by
> >>>ffmpeg -i `seq -f myfile%02g -s '*' 0 12` ...
> >>If spaces in filenames, you're screwed 
> >
> >ls -b ... | tr '\n' '*'
> 
> Well, yes, you can fix anything with a long enough pipe. That does not 
> necessarily mean its not wothwhile implementing the functionality in a 
> more compat way.

iam certainly in favor or a more compact way but IMO interpretation of 
wildcards belongs to the shell, all other unix tools are designed that way too



[...]
> >
> >
> >>And anyway you still could use the shell wildcards if so desired, I 
> >>forgot to give real life examples. The following 4 are quivalent in my 
> >>test tree:
> >
> >its not about being able to do it its about having code in svn which is
> >superfluous, one of the most important goals is to keep the code simple
> >not to include stuff which serves no purpose
> 
> Numbered sequences are in the library, so they can be used from client 
> applications, not necessarily from the command line in the shell. IMO if 
> the wildcard sequences are unacceptable, then the numbered ones should 
> disappear too (for input at least). Is that what you're proposing, to 
> replace numbered by *-delimited?

iam not opposed to remove %d support if the same effect can be achived
differently ...


> 
> And what if the command line becomes overly long for shells not as 
> generouslt parametrized as in Linux?

thats a problem indeed, leaving the @some_url in there seems to be the best
solution


> 
> >
> >[...]
> >
> >>>>/* return -1 if no image found */
> >>>>-static int find_image_range(int *pfirst_index, int *plast_index,
> >>>>-                            const char *path)
> >>>>+static int find_image_number_range(int *pfirst_index, int *plast_index,
> >>>>+                                   const char *path)
> >>>>{
> >>>>    char buf[1024];
> >>>>    int range, last_index, range1, first_index;
> >>>>
> >>>>    /* find the first image */
> >>>>    for(first_index = 0; first_index < 5; first_index++) {
> >>>>-        if (get_frame_filename(buf, sizeof(buf), path, first_index) < 
> >>>>0){
> >>>>+        if (get_frame_filename_number(buf, sizeof(buf), path, 
> >>>>first_index) < 0){
> >>>>            *pfirst_index =
> >>>>-            *plast_index = 1;
> >>>>+                *plast_index = 1;
> >>>>            return 0;
> >>>>        }
> >>>>        if (url_exist(buf))
> >>>>@@ -124,8 +129,8 @@
> >>>>                range1 = 1;
> >>>>            else
> >>>>                range1 = 2 * range;
> >>>>-            if (get_frame_filename(buf, sizeof(buf), path,
> >>>>-                                   last_index + range1) < 0)
> >>>>+            if (get_frame_filename_number(buf, sizeof(buf), path,
> >>>>+                                          last_index + range1) < 0)
> >>>>                goto fail;
> >>>>            if (!url_exist(buf))
> >>>>                break;
> >>>>@@ -142,14 +147,191 @@
> >>>>    *pfirst_index = first_index;
> >>>>    *plast_index = last_index;
> >>>>    return 0;
> >>>>- fail:
> >>>>+  fail:
> >>>>    return -1;
> >>>>}
> >>>cosmetic
> >>Right, and that means it should go in a different patch, will do so at 
> >>once. BTW you missed a similar cosmetic at the line *plast_index = 1; 
> >>above.
> >
> >its not only cosmetic but unacceptable, get_frame_filename() outputs a 
> >filename
> >no number so renaming it to get_frame_filename_number() is just wrong
> 
> get_frame_filename_numbered OK?

ok, but the renaming should be a seperate patch

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is