ConvertBits::Create calls GetFrame

[magiblot]

ConvertBits calls GetFrame while being constructed:

AviSynthPlus/avs_core/convert/convert_bits.cpp

Line 1105 in e613237

auto frame0 = clip->GetFrame(0, env);

Depending on the source clip complexity and the number of ConvertBits instances, this can result in the script taking several minutes to load (which is my case).

The call to GetFrame could be moved inside the if statement. Then it could be avoided by specifying the fulls parameter manually. However, if GetFrame is being called in order to get the frame properties, I ask myself: shouldn't this property be queried for every frame and not just the first one? If this frame property is meant to be constant across the whole clip, shouldn't it be a clip property instead? (I don't know whether such a concept exists, though)

Thank you.

[pinterf]

The call to GetFrame could be moved inside the if statement. Then it could be avoided by specifying the fulls parameter manually.

True, this will be mentioned later ("less ideal" part)

However, if GetFrame is being called in order to get the frame properties, I ask myself: shouldn't this property be queried for
every frame and not just the first one? If this frame property is meant to be constant across the whole clip, shouldn't it be a
clip property instead? (I don't know whether such a concept exists, though)

Yep, I'm not satisfied with the present concept, either.

The following considerations are generic, and do not apply only on ConvertBits.

There is no such object as 'clip properties'. I wish it existed. We could use only some bits for this purpose in the VideoInfo struct.
Most filter assumes constant format across the lifetime of the clip. Some pre-filled variables, sometimes LUTs, or actually used conversion functions for GetFrame can be (ideally) established in filter constructor. (But as you can see, the situation is mixed)
If a filter would like to know the format, governed by one or more frame properties, it must obtain them in filter constructor, calling GetFrame(0).

Possible solutions:

• (less ideal)
  When it is not necessary - e.g. a direct parameter makes it possible - don't call GetFrame(0) from constructor
• (ideal) rewrite such filters one by one to be smart.
  They would behave in a real dynamical way, and work upon actual frame properties; even different ones for each GetFrame.
  Function dispatchers must be inside GetFrame, and no format assumption is stored in a class variable.

Obviously, when a filter constructor contains a relatively long process, e.g. LUT calculation (I think such filters are Tweak, Levels, Expr in LUT mode), and it is based upon certain clip properties they must assume constant format. Or if not: the must use a much smarter way and create LUT only on their first GetFrame, and track changes whether the actual LUT is still valid or must be recreated. This means however that for non-constant properties the LUT may be recrated in each GetFrame (not even considering the possible multithreading situations). It would mean that such filter cannot be a MT_NICE_FILTER because it must keep tracked its internal state.

[magiblot]

There is no such object as 'clip properties'. I wish it existed. We could use only some bits for this purpose in the VideoInfo struct.

This seems true. I don't see a way to add more members to IClip or GenericVideoFilter without breaking ABI compatibility. The only way around this would be to store the clip properties of all clips in a global variable.

(ideal) rewrite such filters one by one to be smart.

But from what I see, we don't need that behaviour in this case: there's little to no benefit in choosing between full/limited range on a per-frame basis. So the performance impact of implementing that outweights the usefulness of it, in my opinion. If that's the only alternative to being inconsistent (using frame properties for what should be clip properties), I prefer to stay inconsistent.

[pinterf]

In this case - ConvertBits - there is no performance penalty I think, one or two more ifs and switch-case put into GetFrame is still quick compared to the actual frame processing.