Imagereader far too slow

[kassandro]

I was stunned, when I measured the builtin Imagereader clip generator with an uncompressed 663x414 bmp image. Obviously Imagereader renders the Image each time it receives a frame request and it must be an extremely slow renderer as well. I only got 7 fps (!!!!!!!). A similar sized mpeg2 clip is rendered by mpeg2dec3 at a speed of about 130 fps. Imagereader should be zero cost. The Image should be rendered only once during initialisation to a VideoFrame and each time a frame request comes in, the same VideoFrame should be returned. Not even a bitblt is necessary. Though Imagereader is not very important, this embarrassment should corrected.

[sh0dan]

You are at liberty to implement an improved version.

[Mug Funky]

hmm... a faster imagereader would be kinda cool.

at the moment i only _ever_ use it for specialised stuff like hand-drawn mattes for compositing, and it works faster than premiere in this case (well... that's shooting fish in a barrel really) with uncompressed tiffs.

definitely not a high priority, as most of the tasks that require it are incredibly slow in their own right (stop motion, hand animation, etc)

@ kassandro: if you could tinker with it we'd all be grateful... you're already in my top 10 list of cool people for "removedirt"... i'm making constant q2 encodes at 500 megs now for high-res movies...

[Nic]

@kassandro: /avisynth/src/sources/ImageSeq.cpp is where you should look, the DevIL library is used to load the image with each call to GetFrame. The least you can do now is modify the source or produce your own ImageReader after so rudely calling the code an "embarrassment".....Shouldn't take long to modify.

However keep in mind, the reason it is slow is because it is also designed to load a series of images (i.e. it starts with a base filename and increments the number contained in that base name to match the next file). So you have to modify it so it loads and stores the most recent image and if the next imagefile in the sequence does not exist just display the previous image from memory.

-Nic

[hanfrunz]

@kassandro: can you please check my importuncompressed filter? The importUncompressedSequence mode="cineon" filter should import over 100 filetypes using an externel lib. If you like to import uncompressed TIFFs or PIXARs you can use the importUncompressedSequence mode="RGB24/32" filter (see readme.txt)
Maybe it's faster, but i recognized, that harddisk-speed is the main problem, not the import filters...

hanfrunz

[kassandro]

I have checked ImageSeq.cpp and ImageSeq.h. Only four lines of code have to be added and altered ImageSeq.cpp compiles well. However, I can't compile Avisynth, because I would have to install the DirectX sdk and that is a 200 Mb monster. Thus somebody else has to test it.
Here are the changes:
The line
PVideoFrame repeat_frame;
has to be added somewhere in the private section of the class declaration of ImageReader (contained in ImageSeq.h).
The line
repeat_frame = NULL;
has to be added somewhere in the constructor of ImageReader, i.e. the function ImageReader::ImageReader(.....) (contained in ImageSeq.cpp).
The line
if( repeat_frame && n >= start && n <= end ) return repeat_frame;
has to be added as the very first line of the function ImageReader::GetFrame(....) (contained in ImageSeq.h).
The line
if( strcmp(fileName, base_name) == 0 ) repeat_frame = frame;
has to be added immediately before the last return statement in function ImageReader::GetFrame(....). It will then be the penultimate line of this function.
With these few changes ImageReader should become a zero cost clip generator, if ImageReader uses only a single picture from disk. There will be no change and no speed improvement, if ImageReader uses a sequence of pictures from disk (Nic already made such a comment).

[kassandro]

I think your filter ImportUncompressedSequence can only be used as a replacement, if ImageReader is used for a sequence of pictures. However, my improvement concerns the other case, when ImageReader is used with a single picture and generates a clip with all frames equal to this picture (see also Nic's comment). I was surprised that ImageReader was so slow in this simple mode.
What makes your filters ImportUncompressed and ImportUncompressedSequence difficult to use are the offset variables, which require internal information about the input material. Because of this reason the "class Preset" of the forthcoming version 0.6 of my plugin RemoveDirt might be useful for your plugin. With the aid of this class, users of your filter need only write once these offsets for theirs formats into a file ImportUncompressedSequence.ini and could reuse these formats simply by specifying ImportUncompressedSequence(default="myformat", ....), where myformat is a name chosen by the user for a certain set of values for all or some of the variables of the filter.

[WarpEnterprises]

- the latest (!) AviSynth compiles without DXSDK

- if you already know that you have only one pic use

ImageReader("...",1,1).Loop(1000,1,1)

- some timings (900MHz Celeron)

Image size: 704x576, 200 images.
bmp 1189kB
jpg 19kB
tga 59kB

ImageSequence tga 14s
jpg 32s
bmp 53s

CoronaSequence tga ---
jpg 14s
bmp 12s

ImageReader tga 9s
jpg 13s
use_devil=false bmp 47s
use_devil=true bmp 12s


As you can see IR (when using devil) is "as fast as possible".

The only things I am missing (maybe only because I needed them and built them in CS/IS):
- NO error throwing when an image is missing (very anoying to me)
- this implies some searching for the first image
- displaying the filename searched for (sprintf is not really everday-language)

BTW CoronaSequence can handle GIF, so one of the 3 possible readers will do the job!

[kassandro]

Originally posted by WarpEnterprises
- the latest (!) AviSynth compiles without DXSDK

Well I get the following error messages (version 2.54):

c:\usr\AviSynth\src\sources\directshow_source.h(41) : fatal error C1083: Include-Datei kann nicht ge”ffnet werden: 'streams.h': No such file or directory
directshow_source.cpp
c:\usr\AviSynth\src\sources\directshow_source.h(41) : fatal error C1083: Include-Datei kann nicht ge”ffnet werden: 'streams.h': No such file or directory



- if you already know that you have only one pic use

ImageReader("...",1,1).Loop(1000,1,1)

That should be zero cost as well. Nice idea to use loop here. Nevertheless ImageReader should work the same way without loop. The necessary changes are not that big.


- some timings (900MHz Celeron)

Well I had use_devil=false and then your timings roughly confirm mine (1.3 GHZ Celeron) with a single image. With use_devil=true I just got 36 fps with a 880 kb bmp file.

[WarpEnterprises]

You don't have the latest source!
It's not available as package so far, only in CVS source DX source has been moved to a "plugin" dir.

Maybe Richard has some spare time ;) to pick up our ideas.

[Richard Berg]

Hi, I'm back.

The Image should be rendered only once during initialisation to a VideoFrame and each time a frame request comes in, the same VideoFrame should be returned.
I'll be grateful and interpret this as a feature request.

Image* was designed as a way to save and restore videos as frame sequences - hence the unusual modes for dealing with raw YUV formats - from a slow interface like a network (original version) or disk. I did some performance testing each time I changed libraries, but always concluded that the interface was the limiting factor when writing uncompressed formats and that the compressed formats were as quick as competing alternatives.

Repeating single images seems to be a popular usage, so I'll add a more friendly way to do so. Most likely via a new parameter and obvious code changes, unless anyone thinks it would be significantly clearer to add a new keyword like StaticImage (which is actually an internal AVS class already).

displaying the filename searched for (sprintf is not really everday-language)
This was fixed in the same CVS version that you pointed Kassandro to :)

I will look into using a MessageClip instead of exceptions for more minor errors, and also finding a library (maybe Corona) with working GIF support.

Finally: is there demand for automatically calling FlipVertical on the formats that the current libs mishandle, or would that break scripts?

[stickboy]

Originally posted by Richard Berg
Repeating single images seems to be a popular usage, so I'll add a more friendly way to do so. Most likely via a new parameter and obvious code changes, unless anyone thinks it would be significantly clearer to add a new keyword like StaticImage (which is actually an internal AVS class already).I don't think separate functions for single images versus image sequences is necessary. Perhaps some additional documentation would be sufficient? Or you could make the "begin" and "end" parameters optional.Finally: is there demand for automatically calling FlipVertical on the formats that the current libs mishandle, or would that break scripts?It would certainly break some stuff, but I still demand the more intuitive behavior of correctly oriented output. I have no problems with breaking things as long as they don't cause silent errors; if AviSynth displays an error message loudly complaining that a script needs some fixing, that's fine with me.

Perhaps the easiest thing to do would be to rename ImageReader? Maybe something like "ImageSource" (that would be more consistent with the AVISource/DirectShowSource/et al. names). Or you could add it as a separate function, keep the existing ImageReader around, and phase it out by removing ImageReader from the docs or by visibly marking it as deprecated.

But if that's something you want to consider, we should plan if other behaviors ought to be changed too.
Originally posted by WarpEnterprises:
The only things I am missing...:
- NO error throwing when an image is missing (very anoying to me)I agree. I'd prefer an optional parameter to control error-handling, perhaps an "ignoreErrors" boolean parameter that defaults to false and that duplicates missing frames if true. Or maybe an "onError" string parameter would be better, where you could pass it arguments such as "duplicate", "skip", "blank", "warn".

I think the fps argument should be a float. It's not clear to me why there are functions that take fps numerators and denominators when they can invoke AssumeFPS.

I also dislike how ImageReader(formatString, 10, 100) generates a 100 frame clip with 10 blank frames at the beginning.

(To summarize, I suppose I wish ImageReader behaved more like my JDL_ImageSource function. :) )

[Richard Berg]

Now in the CVS:
- ImageReader: static image support (just reference a filename directly)
- ImageReader: floating-point FPS
- ImageReader: aliased to ImageSource (error message prefixes unchanged however)
- ImageReader: frames automatically flipped when necessary
- ImageWriter: optional "info" parameter to show filename*
- both: all errors returned as text clips

*this has actually been in there since the beginning, just not documented

It's not clear to me why there are functions that take fps numerators and denominators when they can invoke AssumeFPS.
Well, it's not clear to me why VideoInfo makes us worry about it in the first place :)

I also dislike how ImageReader(formatString, 10, 100) generates a 100 frame clip with 10 blank frames at the beginning.
I suppose you want a 90-frame clip? To me it's more intuitive to have it such that when VDub requests the 50th frame, you get file50.png.

[sh0dan]

@Richard: Great! I'll get a CVS binary out soon!

Well, it's not clear to me why VideoInfo makes us worry about it in the first place
One word: Rounding!

[stickboy]

Originally posted by Richard Berg
- ImageReader: aliased to ImageSource (error message prefixes unchanged however)Is it a direct alias? I'm not confident that's a good idea. What I meant before was that if you want to change ImageReader in such a way that it could silently break existing scripts (like the automatic vertical flip), then those changes would be better in a new function.
I also dislike how ImageReader(formatString, 10, 100) generates a 100 frame clip with 10 blank frames at the beginning.I suppose you want a 90-frame clip? To me it's more intuitive to have it such that when VDub requests the 50th frame, you get file50.png.Yes, I expect a 90-frame clip, not a 100-frame clip. (Edit: Oops, those numbers should be 91 and 101.)

I can see how your way can be convenient too, though. OTOH, those blank frames at the beginning are pretty useless (unless you're using the generated clip as an alpha mask), and they get in the way if you're splicing clips together.

But either way, the behavior can be easily adjusted with a user-defined wrapper function, so no big deal.
- ImageWriter: optional "info" parameter to show filename*Hm, if you want to be able to correlate frame numbers to files with ImageReader, then an option like this for ImageReader might be useful too; then it won't matter what edits are performed on the clip afterward.

[Richard Berg]

Info parameter added to ImageReader. (Not sure why I missed this possibility before.)

I wasn't worried about breaking scripts, as I think anyone good enough to deal with the old flip bug will be able to adapt. The alias is just there for continuity with the other source filters. Mostly, I want ImageSource to get its own page once the wiki is editable again -- there are too many confusable parameters in these two functions to be squished together.

[stickboy]

Originally posted by Richard Berg
I wasn't worried about breaking scripts, as I think anyone good enough to deal with the old flip bug will be able to adapt.How about getting rid of ImageReader completely then? (Or stub it out with env->ThrowError("Use ImageSource instead; see docs, blah blah blah")) :)