Openshot-qt: Rejecting valid image files for image sequence import

[crablab]

When trying to import multiple .jpg files as an image sequence, OpenShot gives an error saying that they are not valid image, sound or video files (which they are).

Image here:

[opensourcegrappler]

I also have this error.

Changing the filename of the image seems to fix it (in my case image000.png to foo.png) allows the image to get imported. But if the image is part of an image sequence (it is in my case) the name change prevents the rest of the image sequence from being imported.

[DylanC]

@crablab - Do you get this error in v2.3.4 or in the daily build?

[opensourcegrappler]

I haven't used openshot since I commented on this issue 1 year ago. So I don't know, sorry.

[DylanC]

@crablab - Image sequence is working much better now. You should try v2.4.

[zerekw]

I'm still getting this error in 2.4 on OSX 10.12.6. It fails the first time and then on trying to reimport the file it successfully imports the single image but not the sequence.

[zerekw]

Must be something with the name. Ran an automator script to modify my filenames from "G0011602.JPG" -> "gopro1.JPG". Now everything seems to be working.

[zerekw]

I suspect its something with the number. When I tried to start from a large number, ie. "gopro1234". It choked up again. When I changed the sequence to start from a file named "gopro1.JPG" it worked.

[DylanC]

@zerekw - Yes, sounds like it was designed to work with simple numbers for now.

[tahteche]

In my case an underscore in the file name seems to have the same effect.

[dougisfunny]

From what I can see, it's not so much 'simple' numbers. They just have to start with 0s.
This works:

Sequence01.jpg
Sequence02.jpg
Sequence03.jpg
Sequence04.jpg

As does this
Sequence04.jpg
Sequence05.jpg
Sequence06.jpg
Sequence07.jpg

This does not

Sequence11.jpg
Sequence12.jpg
Sequence13.jpg
Sequence14.jpg

Nor Does this
Sequence05.jpg
Sequence06.jpg
Sequence07.jpg
Sequence08.jpg

Looking through the code I tracked it to files_treeview.py line ~200

                image_seq = openshot.Clip(os.path.join(folder_path, pattern))

                # Update file details
                file.data["path"] = os.path.join(folder_path, pattern)
                file.data["media_type"] = "video"
                file.data["duration"] = image_seq.Reader().info.duration

By adding a traceback extraction I was able to get this,

Sequence10.jpg is not a valid video, audio, or image file.

No Reader has been initialized for this Clip. Call Reader(*reader) before calling this method.

[, ]

Which I understand is a wrapper for the libopenshot, taking me to Clip.cpp, on line 172 opening a QtImageReader(path) or if that fails the FFmpegReader.cpp at line 178. But that is going a bit far down the rabbit hole, and besides, none of those appear to have anything to do with the image sequence stuff.

Either way, at this point I'm not clear on how, or perhaps more importantly where, the image sequence functionality 'magic' happens to converts the Sequence%02d.jpg to an actual sequence. Or why it only allows numbers lower than 5. So I don't know where it is failing.

[DylanC]

@dougisfunny - Thanks for this extra info. We should look into this further.

[caigner]

I just installed OpenShot on Gentoo Linux and when I start openshot-qt I get a bunch of similar errors.

/usr/lib64/python3.6/site-packages/openshot_qt/effects/icons/bars.png is not a valid image file.
/usr/lib64/python3.6/site-packages/openshot_qt/effects/icons/color shift.png is not a valid image file.
/usr/lib64/python3.6/site-packages/openshot_qt/effects/icons/crop.png is not a valid image file.
...

[mikelacsa]

Tested this with the latest version and it works fine. Closing the issue now.

[ferdnyc]

Tested this with the latest version and it works fine.

Well... it doesn't _entirely_. Some of the things @dougisfunny brought up are still valid.

This does not

Sequence11.jpg
Sequence12.jpg
Sequence13.jpg
Sequence14.jpg

Nor Does this

Sequence05.jpg
Sequence06.jpg
Sequence07.jpg
Sequence08.jpg

See https://github.com/OpenShot/openshot-qt/issues/2947#issuecomment-524672848 for my explanation of why sequences with starting numbers > 4 will fail to import.

Basically, as I said in that issue, it's an FFmpeg limitation, and I'm not sure what we could really _do_ about it, but it does cause unexpected failures for users who assume that any sequence numbering is valid. (Especially since OpenShot treats any sequence numbering as valid, not checking that the numbering starts low enough.)

[jokfish]

I might not understand the issue properly, but is it possible to use the -start_number flag in FFMPEG?
https://ffmpeg.org/faq.html#How-do-I-encode-single-pictures-into-movies_003f
It would also be great if the start number wasn't limited to 0-99. Software such as 3dsMax start the numbering at the current frame number in the software, which can easily be in the thousands.
Is the get_image_sequence_details function in files_listview.py and files_treeview.py the right place to look?

[ferdnyc]

@jokfish Interesting! That's good to know, thanks .One problem is that we don't use the ffmpeg command line, OpenShot is linked directly with the ffmpeg libraries. But, if there's a way to achieve the same thing using the public API, then libopenshot should be able to take advantage of that, yeah.

I've already concluded that reading the API docs isn't going to shed any light on this subject, since every search I attempt just lands me at files full of completely-undocumented functions, with only the name and the arguments to try and puzzle out what they do. But I'll take a look at the source for the ffmpeg command-line tool, see how they implement --start_number. Hopefully it's not relying on internal API calls or anything.

Is the get_image_sequence_details function in files_listview.py and files_treeview.py the right place to look?

It is (also modified by my #2949, not yet merged), as well as its identical copy-paste cousin in files_treeview.py. (...Don't get me started.)

However, it may not be worth it, because if we are going to support start_number then that entire function is going to have to be thrown out; it's far too inefficient.

Currently it loops over potential filenames, based on the selected one, doing an individual os.path.exists() call for _each and every_ possible match. Fortunately it starts at the selected number - 100, which explains the limitation but is also why files that are numbered "in the thousands" as you say can't completely hang the import process.

If OpenShot is going to have to compute the starting point of an arbitrarily-positioned run of numbers in a set of files, though, looping over os.path.exists() is madness. A whole new method will have to be written that reads in the complete directory listing as a list, then performs list bonsai to produce just the set of filenames required.

But if you're in the mood to _write_ that new method, then by all means! (Just... maybe buck the trend and write it _once_ in src/windows/models/files_model.py, where it BELONGS.)

[ferdnyc]

(If I had my way we wouldn't even bother with all of this terrible code just trying to tease arbitrary sets of filenames out of a directory full of random who-knows-what. Image sequences would be required to live in their own separate directories, one sequence per, and no other files would be permitted to be in there. Then, we could just sort the directory listing numerically, take the first entry, and vioila, there's your start_number.)

[jokfish]

Excellent, I'll take a look when I get a chance. Might just take me a little while to get an idea of the current code.
Some thoughts;

• Would it be unreasonable to require that the user selects the first image in the sequence? - That would make processing un-zero-padded sequences a lot easier for one, and actually gives more flexibility (to load a sequence starting in the middle, or skipping the first few frames).
  This also gives the start_number directly.
• The sequence should stop at the first missing number or gap - It's likely that this is desired behaviour for a couple of reasons. If the missing frames are a mistake it highlights missing frames before the final render - a check of sorts. If the missing frames are deliberate it enables the creation of two sequences, which can be inserted separately, which is probably what the user wanted.

[ferdnyc]

Oh, this doesn't look too bad, actually. start_number is one of the muxing parameters used in libavutil/img2enc.c(L210:L219):

#define OFFSET(x) offsetof(VideoMuxData, x)
#define ENC AV_OPT_FLAG_ENCODING_PARAM
static const AVOption muxoptions[] = {
    { "update",       "continuously overwrite one file", OFFSET(update),  AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0,       1, ENC },
    { "start_number", "set first number in the sequence", OFFSET(img_number), AV_OPT_TYPE_INT,  { .i64 = 1 }, 0, INT_MAX, ENC },
    { "strftime",     "use strftime for filename", OFFSET(use_strftime),  AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, ENC },
    { "frame_pts",    "use current frame pts for filename", OFFSET(frame_pts),  AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, ENC },
    { "atomic_writing", "write files atomically (using temporary files and renames)", OFFSET(use_rename), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, ENC },
    { NULL },
};

...Oh, shit. Except, it's _not_ part of the public API. This is what I was afraid of.

$ find /usr/include/ffmpeg -type f -print0 |xargs -0 -n100 grep -i start_num
$

So, that's a complication, unfortunately.

Still, I'll keep looking for solutions. Maybe we can just duplicate the AVOption struct from img2enc.c in our code. The AVOption type itself _is_ part of the public API, and so is AV_OPT_FLAG_ENCODING_PARAM... the rest of it is just string constants, so maybe that's everything we really need.

[ferdnyc]

• The sequence should stop at the first missing number or gap - It's likely that this is desired behaviour for a couple of reasons.

Not just desired behavior, it's a hard requirement, yeah. FFmpeg _will_ stop as soon as the continuous numbers run out, so there's no choice in the matter.

• Would it be unreasonable to require that the user selects the first image in the sequence? - That would make processing un-zero-padded sequences a lot easier for one, and actually gives more flexibility (to load a sequence starting in the middle, or skipping the first few frames).
  This also gives the start_number directly.

Personally I don't think _anything's_ unreasonable (you have no idea how much I hate this feature), but that _would_ be a big change in functionality from what current users are used to, which might be problematic. ...Might not, I don't know, I'd have to find out how users would react to such a change.

But... TBH, setting that restriction really wouldn't buy you anything significant in terms of code simplicity or anything, now that I think about it. Here's why...

Walking the list

Even _if_ we were to require that the user select the first frame in the sequence, OpenShot STILL has to walk the list of files. That's so that it can both:

1. Determine whether there _is_ indeed an image sequence present. (At least three contiguous numbered filenames are required to qualify as a possibility. And keep in mind, there are _plenty_ of times a user will select a file title_01.jpg to import, from a directory that also contains title_bg.jpg, title2_02.jpg, title002.png, and frame_02.jpg. There's no possible image sequence there, and OpenShot needs to detect that and not offer.)
2. Also, it needs the opposite boundary of the sequence's image list, so that it knows the length in frames to assign to the imported clip created from that sequence. So, once you've committed to walking the list in _one_ direction regardless, walking it in _both_ directions barely adds any complexity, really.

(And despite the horrible method the _current_ code uses for doing this, if you have a complete list of the filenames, generating the prefix is actually really easy. All you need is the filename from each end of the list.

Pattern generation

Like, say you've determined that your image sequence list is a list of 90 files:

• myanim2_0021.jpg
• myanim2_0022.jpg
• ...
• myanim2_0110.jpg

To generate the sequence pattern (in a _sane_ manner), all you need are the start filename myanim2_0021.jpg and the end filename myanim2_0110.jpg.

1. First you determine whatever prefix string they have in common. (myanim2_0, in this case)
2. So you chop that prefix off of either filename. (myanim2_0021.jpg => 021.jpg)
3. Also drop the extension. (021.jpg => 021)
4. So now that what's left is entirely numeric, you take its length in digits N. (021 => 3)
5. A printf-style, 0-padded format string, %0Nd, replaces the numeric part. (021 => %03d)
6. And voila, your pattern is prefix + format_string + extension, or myanim2_0%03d.jpg.

If you implemented the algorithm _differently_, so that it came up with a pattern of myanim2_%04d.jpg instead? Doesn't matter! If you're passing the start_number to FFmpeg, the exact pattern structure is irrelevant just as long as it's valid.

Pattern flexibility

Even if the user generated those same filenames as myanim20021.jpg through myanim20110.jpg, so you couldn't easily tell which part of the filename is the name of the sequence and which part is the incremental frame-numbering... it actually _doesn't matter_. Whether you generate the pattern as:

• myanim20%03d.jpg with start_number = 21
• myanim2%04d.jpg with start_number = 21
• myanim%05d.jpg with start_number = 20021

... all three would work _exactly_ the same. It simply does not matter, _as long as you can specify the start_number_.

The current code attempts to "guess" the pattern up front, based on the filename it's initially passed, and then scans the directory to see if it contains any other filenames that match that assumptive pattern. It's not _just_ the fact that it's so inefficient and error-prone, though it is. (Part of the reason the feature sucks so hard, and makes me hate it so much.) But that's actually the _harder_ approach, because it's completely bass-ackwards.

A far simpler approach is:

List bonsai

1. Grab the list of filenames in the directory, sorted numerically (based on the trailing edge of each name, not counting the extension).
2. Find the initially-selected filename's position on that list.
3. Starting from that position, walk the list in both directions.
4. At each step, check whether the next adjacent entry is the previous name with 1 either added or subtracted from it (again, at the least significant digit position), depending which direction.
5. The first filename you encounter that _isn't_ the previous name ±1, you've gone past end of the sequence. Discard that item, _and_ all remaining items in that direction.
6. Once you've determined the start and end filenames in the continuous list, and if the length of that list is >= 3, apply the prefix-trimming algorithm I laid out earlier to convert the names into any valid pattern with corresponding start_number.

Matching extensions

Oh, yeah, and _one_ last wrinkle: The files in the sequence list all have to have the same extension as well. slide1_001.jpg, slide2_002.png, and slide3_003.jpg could never be an image sequence, since the non-uniform encoding would crash libavformat.

(In truth, to avoid crashing all of the files have to have _exactly_ the same image properties: Same encoding, same pixel dimensions, same color depth, etc. Unfortunately, as I noted in #3044, OpenShot can't actually _ensure_ they all match without opening each and every image file in the sequence, one by one. Which would be way too expensive an operation, for far too little reward.

[mohamed-aiman]

I faced this issue today. Just selected all files and renamed with Sequence and it worked. The output name format was.

Sequence(1).jpg
Sequence(2).jpg
Sequence(3).jpg

[sg-github-account]

I got this issue using openshot version 2.5.1 (libopenshot0.2.5) on Linux mint.
File names were

DSCN0977.JPG DSCN0990.JPG DSCN1003.JPG DSCN1016.JPG DSCN1029.JPG DSCN1042.JPG DSCN1055.JPG DSCN1068.JPG DSCN1081.JPG DSCN1094.JPG DSCN1108.JPG DSCN1121.JPG
DSCN0978.JPG DSCN0991.JPG DSCN1004.JPG DSCN1017.JPG DSCN1030.JPG DSCN1043.JPG DSCN1056.JPG DSCN1069.JPG DSCN1082.JPG DSCN1095.JPG DSCN1109.JPG DSCN1122.JPG
DSCN0979.JPG DSCN0992.JPG DSCN1005.JPG DSCN1018.JPG DSCN1031.JPG DSCN1044.JPG DSCN1057.JPG DSCN1070.JPG DSCN1083.JPG DSCN1096.JPG DSCN1110.JPG
DSCN0980.JPG DSCN0993.JPG DSCN1006.JPG DSCN1019.JPG DSCN1032.JPG DSCN1045.JPG DSCN1058.JPG DSCN1071.JPG DSCN1084.JPG DSCN1097.JPG DSCN1111.JPG
DSCN0981.JPG DSCN0994.JPG DSCN1007.JPG DSCN1020.JPG DSCN1033.JPG DSCN1046.JPG DSCN1059.JPG DSCN1072.JPG DSCN1085.JPG DSCN1098.JPG DSCN1112.JPG

After a few hours I finally got it working by renaming / soft linking to

ln -s ../hail/DSCN0977.JPG 001.jpg
ln -s ../hail/DSCN0978.JPG 002.jpg
ln -s ../hail/DSCN0979.JPG 003.jpg
ln -s ../hail/DSCN0980.JPG 004.jpg
ln -s ../hail/DSCN0981.JPG 005.jpg
ln -s ../hail/DSCN0982.JPG 006.jpg
ln -s ../hail/DSCN0983.JPG 007.jpg
ln -s ../hail/DSCN0984.JPG 008.jpg
ln -s ../hail/DSCN0985.JPG 009.jpg

The code generation to rename all the files used awk and NR option

ls -1R | awk '{print "ln -s ../hail/"$0" 00"NR".jpg"}' >script1.txt

[sg-github-account]

With regards to the "The code generation to rename all the files used awk and NR option"

ls -1R | awk '{print "ln -s ../hail/"$0" 00"NR".jpg"}' >script1.txt

you will need to add a 0 to any first 9 files ie

ln -s ../hail/DSCN0977.JPG 001.jpg
becomes
ln -s ../hail/DSCN0977.JPG 0001.jpg

otherwise 0010.jpg+ will not be loaded.