[Ffmpeg-devel] [PATCH] Wildcard and catalog image sequences
Michael Niedermayer
michaelni
Tue Aug 29 23:54:23 CEST 2006
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` ...
anyway, review of your patch is below, iam unsure if i will or will not apply
it if you fix all below but dont leave the wildcard handling to the shell
to which it IMO belongs ... comments are of course welcome
[...]
[configure part left to maintainer]
[...]
> /* 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
>
> +static void free_catalog(int last_index, char*** pindex_vector)
> +{
> + int i;
> + char** index_vector = *pindex_vector;
> + for(i=0;index_vector&&i<=last_index;i++)
> + av_free(index_vector[i]);
> + av_free(index_vector);
> + *pindex_vector = NULL;
av_freep()
and the index_vector variable seems unneeded
> +}
>
> +static int amatch(const char* s, const char* p, char matchZeroOrMore, char matchOne,
> + int case_sensitive)
> +{
> + int c, scc;
> +
> + for (;;) {
> + scc = *s++;
> + c = *p++;
> + if(c==matchZeroOrMore) {
> + if (!*p)
> + return (1);
the () is unneeded
> + for (s--; *s; s++)
> + if (amatch(s, p, matchZeroOrMore, matchOne, case_sensitive))
> + return (1);
> + return (0);
> + } else if(c==0) {
> + return (scc == 0);
> + } else if(c==matchOne) {
> + if (scc == 0)
> + return (0);
> + continue;
> + } else if(case_sensitive) {
> + if (c != scc)
> + return (0);
> + continue;
> + } else {
> + if (toupper(c) != toupper(scc))
> + return (0);
> + continue;
the 3 continues do nothing
[...]
> +/* return -1 if no image found */
> +static int find_image_catalog_range(int *pfirst_index, int *plast_index, char*** pindex_vector,
> + const char *path)
the name sucks, this function does more then just finding the range, it loads
the whole thing
> +{
> + char buf[1024];
> + int last_index = (-1), n_total, fileopen = 0;
> + char** catalog = NULL;
> + ByteIOContext bic;
> +
> + if(strcmp(path, "@-")==0) {
> + if(url_fopen(&bic, "pipe:", URL_RDONLY)<0)
> + goto fail;
> + } else {
> + if(url_fopen(&bic, path+1, URL_RDONLY)<0)
> + goto fail;
> + }
char *filename= strcmp(path, "@-") ? path+1 : "pipe:";
if(url_fopen(&bic, filename, URL_RDONLY)<0)
goto fail;
> + fileopen = 1;
> +
> + catalog = av_mallocz(sizeof(char*)*(n_total=128));
> +
> + while (url_fgets(&bic, buf, sizeof(buf))) {
> + while (isspace(buf[strlen(buf)-1]))
> + buf[strlen(buf)-1]=0;
wont that fail if the string is made of only spaces?
[...]
> Index: libavformat/utils.c
> ===================================================================
> --- libavformat/utils.c (revision 6123)
> +++ libavformat/utils.c (working copy)
[...]
> +int filename_wildcard_test(const char *filename)
> +{
> + if(!filename)
> + return (-1);
> + else if(strchr(filename, '*') || strchr(filename, '?'))
> + return 0;
> + else
> + return (-1);
return filename && (strchr(filename, '*') || strchr(filename, '?'));
> +}
> +
> +int filename_catalog_test(const char *filename)
> +{
> + if(!filename)
> + return (-1);
> + else if(filename[0]=='@')
> + return 0;
> + else
> + return (-1);
return filename && filename[0]=='@';
> +}
> +
> +int filename_multiple_test(const char *filename)
> +{
> + if (filename_number_test(filename)==0 ||
> + filename_wildcard_test(filename)==0 ||
> + filename_catalog_test(filename)==0)
> + return 0;
> + else
> + return (-1);
> +}
return filename_number_test (filename)
|| filename_wildcard_test(filename)
|| filename_catalog_test (filename);
[...]
> @@ -3012,6 +3043,38 @@
> }
>
> /**
> + * Returns in 'buf' the nth path in the catalog
> + */
> +int get_frame_filename_catalog(char *buf, int buf_size,
> + char** catalog, int catsize, int number)
> +{
> + size_t len;
> + char* p = catalog[number];
> + if(number >= catsize)
> + goto fail;
> + len = strlen(p)+1;
> + if(len>buf_size)
> + goto fail;
> + memcpy(buf, p, len);
> + return 0;
> + fail:
> + *buf = '\0';
> + return -1;
> +}
> +
> +int get_frame_filename_multiple(char *buf, int buf_size,
> + const char* path, char** catalog, int catsize, int number)
non static functions should have some prefix to avoid conflicts with other libs
[...]
--
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
More information about the ffmpeg-devel
mailing list