Implement basic data model

[mgaitan]

/This is a very raw proof of concept that try to explore a more pythonic API for basic clip manipulation using few of the "dunder" methods (aka python's data model).

In [2]: v = VideoFileClip('/home/tin/Videos/Screencast 2019-12-10 09:56:22.mp4')                                                                                                              

In [3]: v[-2.3:]         #   last 2.3 seconds                                                                                                                                     
Out[3]: <moviepy.video.io.VideoFileClip.VideoFileClip at 0x7fdce65ac1d0>

In [4]: _.duration                                                                                                                                                                            
Out[4]: 2.3000000000000007

In [5]: v[:1, -1:]      # first second + last second.  same than v[:1] +  v[-1:]                                                                                                                                                                           
Out[5]: <moviepy.video.VideoClip.VideoClip at 0x7fdce65a2c90>

In [6]: _.duration                                                                                                                                                                            
Out[6]: 2.0

Extending slicing syntax is supported with speed is supported. See docstring of __getattr__.

The idea is to simplify the most common tasks, not to be cryptic but practical.

other methods implemented are

• __add__ to concatenate (ie clip1 + clip2 == concatenate_videoclips([clip1, clip2]))
• __mul__ for repetition (ie, clip * 3)
• __and__ for masking
• __matmul__ for rotation ( clip @ 270)
• __or__ for simple horizontal union (clip1 | clip2)
• __truediv__ for simple vertical union (clip1 / clip2)

[coveralls]

Coverage: 84.749% (+0.2%) from 84.534% when pulling 21cbf32 on mgaitan:data_model into 9ebabda on Zulko:master.

[tburrows13]

I like this a lot, and I think it should definitely be merged at some point, possibly into an upcoming v2.0 (I've got some other plans as well).

I'd be in favour of __neg__ to reverse a clip, __mul__ to repeat it. __and__ for masks seems fun as well, and would just call myclip.set_mask(maskclip).

[mgaitan]

excellent @tburrows13 . I'll continue working on this on my spare time. Meanwhile, in order to simplify its review, would be great to have #1077 merged before. I'll be glad if you could check that

[tburrows13]

I'd like to merge this one, but I think you need to merge master into it first to resolve the conflicts.

@Zulko @mgaitan do you think that we should encourage the use of this syntax (in particular clip1 + clip2 for concatenation) in things like 'getting started' examples, or just keep it as an optional extra?

Also I think that the concatenation here should possibly use method='compose' instead of 'chain' as it is more intuitive?

[Zulko]

I'm not against this commit because I like experimentations and that looks very fun, but from a programming and maintenance standpoint overloading operators for objects as complex as videoclips could cause confusion. It will make code harder to read for people who don't know what these operators mean, and I expect that many people will be asking for support on these. For instance should "+" mean concatenation or overlay? Why use -clip for time mirroring instead of clip[::-1]? What happens when you just add two clips with different sizes? I like the subclipping with __getitems__though, I think it is long due.

[mgaitan]

I'll update this branch as soon as possible so we could discuss its scope again.

Of course, I understand it's a bit sensitive, and need to be a well document and limited subset of features: the ones that are used more frequently and feels "similar" to behaviours already present in python structures. Concatenation, repetition, slicing, length, masking, etc are common operations supported in builtin types and popular libraries like numpy, with a common syntax.

For instance, the slicing support I've implemented is clip[start:stop:speed], so
clip[::-1] is actually a time_mirror, like reverse any sequence.

My goal is be pragmatic and pythonic, and I would like as much feedback as possible.

[mgaitan]

The PR is ready to review. I've documented the most important method __getattr__ but not sure where to put that text (or a similar one) in the documentation.

[Ethkuil]

It will make code harder to read for people who don't know what these operators mean, and I expect that many people will be asking for support on these. For instance should "+" mean concatenation or overlay?

We can naturally treat a "clip" like a "str", and almost nobody will be confused by why '1'+'1' equals to '11' but not '2'.

[Ethkuil]

Currently it's 2023 and so excuse me, any progress? Or anything needs help?

[mgaitan]

@Ethkuil hi, I still consider this approach would be useful for several use cases but I felt that @Zulko wasn't totally convinced on my approach and then nobody clarified which "magic methods" would be acceptable or not.

I'll be happy to resume and finish this work if there is consensus on what would be considered too much

cc / @Ethkuil @tburrows13

[mgaitan]

I can't really understand this test nor its regression, as it's not affected by my new methods . It was failing like this

E       AssertionError: Expected (7f,7f,00) but got (00,80,80) at timestamp 0.5
E       assert not True

which is the same error that I get in master if I apply this diff to the test

(lab) tin@morocha:~/lab/moviepy$ git diff
diff --git a/tests/test_compositing.py b/tests/test_compositing.py
index 5824034..d3bdee2 100644
--- a/tests/test_compositing.py
+++ b/tests/test_compositing.py
@@ -105,6 +105,7 @@ def test_blit_with_opacity():
         .with_opacity(0.5)
     )
     composite = CompositeVideoClip([clip1, clip2])
+    composite.write_videofile("compo.mp4")
     bt = ClipPixelTest(composite)
 
     bt.expect_color_at(0.5, (0x7F, 0x7F, 0x00))

ie,write de composed videofile before the assert, it shouldn't affect, right? what's happening?

As an alternative, I rewrote the test as I understood below using ColorClip. Is it correct?

[mgaitan]

@Ethkuil @tburrows13 @keikoro this is ready to review,

[keikoro]

@Ethkuil @tburrows13 @keikoro this is ready to review,

Could you provide some context for / update regarding these changes? I've only skimmed the existing conversation and it's unclear to me where we stand on this PR. Older comments seem to suggest no definite decision had been made on if these changes should be included in v2. Thanks!

[mgaitan]

Hi @keikoro, one of the reasons why I had abandoned this proposal was this very lack of definition. I was not sure that my work would be accepted. However, it still seems to me a good idea especially for the basic cases of slicing and concatenation.

When @Ethkui asked if I was thinking of "rounding up" the PR I realized that not much was missing and coincidentally I saw ChatGPT generated a code that would be much more pythonic with these features.

Therefore, I would love to see it included as part of version 2.0.

I have also added some tests that I consider to be better than several of the pre-existing ones (for example the time mirroring case via slicing with a negative step).

If you agree with the general design and the semantics of the included operations, we could write a few paragraphs in the doc. If you have observations or other ideas, I am willing to listen.

[mgaitan]

@keikoro could you try this and give me your impressions?

[mgaitan]

hey, Could I merge this?