Implement basic data model #1076

mgaitan · 2020-02-09T05:06:20Z

/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 · 2020-02-09T05:13:36Z

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

tburrows13 · 2020-02-19T02:04:03Z

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 · 2020-02-19T11:51:35Z

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 · 2020-03-26T13:31:32Z

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 · 2020-03-26T13:41:31Z

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 · 2020-03-27T05:30:50Z

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.

moviepy/Clip.py

mgaitan · 2020-03-28T23:23:38Z

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 · 2023-01-29T00:10:34Z

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 · 2023-01-29T00:12:52Z

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

mgaitan · 2023-02-09T19:12:21Z

@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 · 2023-04-02T16:09:08Z

tests/test_compositing.py

@@ -93,22 +93,44 @@ def test_concatenate_floating_point(util):
    concat.write_videofile(os.path.join(util.TMP_DIR, "concat.mp4"), preset="ultrafast")


+# def test_blit_with_opacity():


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 · 2023-04-03T02:16:27Z

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

keikoro · 2023-04-12T20:42:56Z

@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 · 2023-04-12T21:40:45Z

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 · 2023-04-22T12:29:58Z

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

mgaitan · 2023-05-13T17:48:05Z

hey, Could I merge this?

mgaitan mentioned this pull request Feb 9, 2020

add __getitem__ method to Clip #895

Closed

5 tasks

This comment was marked as resolved.

Sign in to view

mgaitan force-pushed the data_model branch from cdd30b1 to ee6bcc2 Compare February 17, 2020 12:34

tburrows13 added the WIP label Feb 20, 2020

tburrows13 mentioned this pull request Feb 22, 2020

Moviepy Roadmap - version 2.0 #1089

Open

7 tasks

tburrows13 added this to the Release v2.0.0 milestone Feb 23, 2020

tburrows13 added the feature New addition to the API i.e. a new class, method or parameter. label Feb 24, 2020

mgaitan mentioned this pull request Mar 6, 2020

Make effects be callable classes #1096

Closed

tburrows13 removed the WIP label Mar 10, 2020

tburrows13 changed the base branch from master to v2 March 20, 2020 18:43

tburrows13 changed the base branch from v2 to master March 26, 2020 13:00

mgaitan changed the base branch from master to v2 March 27, 2020 18:55

mgaitan mentioned this pull request Mar 28, 2020

Assertions in tests are very fragile #1114

Closed

mgaitan force-pushed the data_model branch from 3d5c390 to cd3d8fe Compare March 28, 2020 23:00

mgaitan commented Mar 28, 2020

View reviewed changes

moviepy/Clip.py Outdated Show resolved Hide resolved

mgaitan changed the title ~~[PoC] implement basic data model~~ Implement basic data model Mar 28, 2020

mgaitan requested review from tburrows13, Zulko and keikoro March 28, 2020 23:26

tburrows13 closed this Mar 29, 2020

tburrows13 reopened this Mar 30, 2020

tburrows13 changed the base branch from v2 to master March 30, 2020 11:33

apply patch

a31a564

tburrows13 added this to the Post 2.0.0 milestone Sep 2, 2020

tburrows13 modified the milestones: Post 2.0.0, Release v2.0.0 Oct 8, 2020

tburrows13 mentioned this pull request Oct 16, 2020

time_mirror fix #1356

Open

4 tasks

mgaitan added 7 commits March 30, 2023 09:28

Merge remote-tracking branch 'upstream/master' into data_model

aa6f83c

lint

36bdf1e

fix time mirror

3ef6219

undo change

4b259f4

merge Zulko#1943

eb55938

undo change

215fe1c

new implementation for test_blit_with_opacity

9d43dd6

mgaitan commented Apr 2, 2023

View reviewed changes

mgaitan added 5 commits April 2, 2023 20:20

add more tests for slicing operations

728dde7

implement __mul__ using loop function as recommended

b8b6c07

or and div

18f4ebb

clip @ angle is a rotation

f4775b3

linters happy

21cbf32

mgaitan force-pushed the data_model branch from c84661f to 21cbf32 Compare April 3, 2023 02:15

mgaitan mentioned this pull request May 24, 2023

Project quality? #1976

Closed

mgaitan merged commit 57c279a into Zulko:master May 26, 2023

mgaitan mentioned this pull request Jun 30, 2023

The Future of MoviePy project #1874

Open

mgaitan mentioned this pull request Mar 4, 2024

V2.0 update for code & documentation #2024

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement basic data model #1076

Implement basic data model #1076

This comment was marked as resolved.

		@@ -93,22 +93,44 @@ def test_concatenate_floating_point(util):
		concat.write_videofile(os.path.join(util.TMP_DIR, "concat.mp4"), preset="ultrafast")


		# def test_blit_with_opacity():

Implement basic data model #1076

Implement basic data model #1076

Conversation

This comment was marked as resolved.

Choose a reason for hiding this comment