Plugin: Turn

[WarpEnterprises]

one missing basic filter (mainly along with importing photos):

TurnLeft and TurnRight

(only AviSynth 2.0x and RGB so far)

[Bidoche]

Yes it was missing.
To do it, you add to import from vdub, a waste for such a basic feature...

I was planning to add into 3.0 core (well, I still am ).

Looking at your code, I see you were stuck by the same problem as I:
How do I turn a YUY2 VideoFrame !?

Edit: my mistake: you had to DO it with vdub...

[sh0dan]

If you're ok with it, I'll add this to the 2.5 core, and add YV12 - that way we won't have any conflicts.

Had it planned for some time for 2.5, but never got around to it. Perhaps Turn180() should also be implemented - at least as an alias for flipvertical().fliphorizontal().

I should perhaps also have a look at redesigning Layer for 2.5 (it should be possible to make more readable/maintainable code).

[WarpEnterprises]

Of course, although the code is quite ugly (you know, I'm new @ C++) but it works and is surprisingly fast although it is no way optimized.

[Bidoche]

Well, everyone is new at C++ here, sh0dan is, you are, I am...

Finally, we can't turn YUY2 VideoFrame....

[Belgabor]

Well, you can, but it would be like ConvertToYV12().Turn().ConvertToYUY2()

[WarpEnterprises]

Bug corrected (thanks to Lyptus).

download

[Kurosu]

What I think could be a core function now works for RGB24/RGB32/YUV2/YV12/I420. I forgot YUV format.

I don't see how it could be optimized to used parallel instruction sets. But it can be added very easily to the plugin thanks to a little rewriting.

Source code is included (it's a GPL project after all) in the package. I optimized a bit the C code (10% more speed ?), but is now less readable.

WarpEnterprises, if you feel like cheated on this, I will delete the attachment.

[WarpEnterprises]

Of course, keep it! My main intention was that there is working code that can be added to the core - and now there is

BTW mine is for 2.0 yours for 2.5

[Bidoche]

Damn ! Why all of you always want to do classes who do everything...

Here you should do one PClip subclass for each case (TurnLeftRGB, TurnRightRGB, TurnLeftYV12 ....)

Calling functions with a direction parameter and use it inside to choose between the two case is not the best option (not even a signle line of code shared between the two branch....)
Those choices should be done at the higher level possible, ie in the Create Function of Turn, and when we are at it, we divide into colorspace too...
It will make code much more understandable.

NB: don't take it for yourself, it's just the core is full of that and it makes things harder to understand.

[sh0dan]

@Bidoche: What difference does it make - it works, and the code is fairly clear?

@Kurosu:
Why do you test:

else if (vi.IsYV12()) {
  if (vi.height%2) env->ThrowError("Turn: YUY2 data must have MOD2 height");
  if (vi.width%2) env->ThrowError("Turn: YUY2 data must have MOD2 width");
  TurnPlanFunc = TurnYV12;
}

YV12 will always be mod2 in both width and height.

But otherwise, it's very nice. I'll add the code ASAP.

[Kurosu]

@Shodan
Because I was first only accepting MOD8 resolution. What surprises me is that YV12 data can actually be processed, while I thought the U field data was interleaved, like this way:

U planar:
line 0 + line 2 U
line 1 + line 3 U
...

It seems to work fine, but it would have been a nightmare otherwise.

@Bidoche
I think the main problem of the new APIs are that no older plugin can be reused without a recompile. That's what keeps Gabest from building a public avs 2.5 textsub filter (afaik). In what I've seen from DCOM in DirectX, it would be pretty usefull for the plugin developpers.

@WarpEnterprises
Thanks. GPL at the work

[sh0dan]

@Kurosu: No - thank god it's not - you could actually use same loop for all three planes - but - again - there is no difference in the real world.

[Bidoche]

@sh0dan

I'd rather say: it works and it's relatively clear (in its case, some piece in the core are far uglier).
Anyway it's still a design flaw to consistantly test colorspace to branch when you could have done it in Turn::Create with some polymorphism.

And for now, the number or colorspace is not that big.
But imagine if there are 10 of them, what it'd be :

if (colorspace1) do process1;
else if (colorspace2) do process2;
else if....
else if (colorspace10) do process10;

or just a switch in the create calling diffent subclass (for code sharing we use inheritance)

The second will produce much more cleaner code (and save a recurrent test at the cost of a virtual table).
For maintaining large amount of code, readibility is important. Imagine you discover that Turn is buggy in RGB24, isn't it better to use the class list of VC6 to go to source rather that dig into the Turn global code searching for the RGB24 code path....

@Kurosu

What is the connection ? I don't get your point...
If you're saying that Turn is a valuable addition, I am totally ok with that.
In fact I am willing to make it VideoFrame method (with some others things) to make easier to plugins developpers to develop effects.


NB: I think maybe turn left and right can be done with the same code using either positive or negative pitch (should be checked)

[Kurosu]

@Bidoche

I didn't see your later post, and I therefore edited my post. I'll try to elaborate. The way the Avisynth API is now oblige the plugin developpers to offer their plugn recompiled for all major API changes, like AVS2.0x and AVS2.5. That's troublesome, as nothing in the filter has changed. Some developpers (like Gabest) have proposed some solution. Still, it's the plugin that tackle the problem, while I think it should be the environment/API/core. It saves some trouble to the plugin writers (see the perplexity of most of us about the name space thingie proposed by Gabest). And it offers a clean solution, once and for all, with no particular work or 'trick' to do by the plugin writer.

I take for example DirectX, where the interface is instanciated and refers to precise feature. If you want maximum compatibility, you can use the older interfaces, which will still be supported by any new API, but would lack some advanced features. But this way, DirectX 9 can still run DirectX 3 games for instance.

In the end, the way we write code depends on the way we master C++. I've learned how to write AVS plugin by reading source code from other people, and I'm only slowly getting some things that are subtilities for me. In the end, we don't really want to bother with them but rather spend time on optimizations/ additionnal features.

I could have written more specific functions, like TurnRightYUY2 , TurnLeftYUY2, ... but it only saves me a simple 'if' at the upper level of the functions (which doesn't hurt speed this way, I believe). I did it for the various colorspaces, because it was saving some bigger 'if' code blocks.

[Bidoche]

@Kurosu

I am not really aware of problems related to the API offered to exterior plugins. But I fear it was not really made easily extensible from the start....


Don't believe I go hunt plugins authors about their code not being that clear, but I would about core code (at least in 3.0)
And I understand that many learn by mimicking existing code so someone has to start giving the good example


And my point about moving the if upwards is:
your way : you make an if per GetFrame call
my way : you test only once in Create and then it's definitely branched ok.

[Kurosu]

@Sh0dan
About YV12 interlacing, then I have trouble understanding the corresponding page on avisynth.org documentation (in addition to my post being not very clear):
http://www.avisynth.org/index.php?page=ColorSpaces

@Bidoche
In the end, I have no idea how to work with the PClip subclasses. I think the function pointers handle this in a more Cish-way, and do the job. Doing what you suggest (at the higher level possible, still not the Create function), I would just add additionnal specific functions

For the negative pitch trick, it has also to work with the start and end values of the loop iterator. If it needs a non-fixed step (either +1 or -1), I think that keeping separate blocks for each case (Right or Left) is better.

[Guest]

@Bidoche

I'm a C embedded systems programmer and not very familiar with C++ idioms. Can you please explain your idea with a simple example? I.e., how would I use polymorphism in the way that you described (instantiating different code in the Create)? Thank you.

[WarpEnterprises]

Yes, an example would be helpful.
This could help to elaborate the possible ways to deal with e.g. many colorspaces or sample sizes ...

[Bidoche]

Ok I will rewrite it.