Test left, right, top, bottom and color arguments of margin FX (#1579)

* Test left, right, top, bottom and color arguments of margin FX

* Format properly margin FX docstring

* Test margin FX using ColorClip

moviepy/video/fx/margin.py

@@ -18,16 +18,30 @@ def margin(
     """
     Draws an external margin all around the frame.
 
-    :param margin_size: if not ``None``, then the new clip has a margin_size of
-        size ``margin_size`` in pixels on the left, right, top, and bottom.
-    :param left, right, top, bottom: width of the margin in pixel
-        in these directions.
+    Parameters
+    ----------
 
-    :param color: color of the margin.
+    margin_size : int, optional
+      If not ``None``, then the new clip has a margin size of
+      size ``margin_size`` in pixels on the left, right, top, and bottom.
 
-    :param mask_margin: value of the mask on the margin. Setting
-        this value to 0 yields transparent margins.
+    left : int, optional
+      If ``margin_size=None``, margin size for the new clip in left direction.
 
+    right : int, optional
+      If ``margin_size=None``, margin size for the new clip in right direction.
+
+    top : int, optional
+      If ``margin_size=None``, margin size for the new clip in top direction.
+
+    bottom : int, optional
+      If ``margin_size=None``, margin size for the new clip in bottom direction.
+
+    color : tuple, optional
+      Color of the margin.
+
+    opacity : float, optional
+      Opacity of the margin. Setting this value to 0 yields transparent margins.
     """
     if (opacity != 1.0) and (clip.mask is None) and not (clip.is_mask):
         clip = clip.add_mask()
@@ -52,8 +66,8 @@ def make_bg(w, h):
 
     else:
 
-        def filter(gf, t):
-            pic = gf(t)
+        def filter(get_frame, t):
+            pic = get_frame(t)
             h, w = pic.shape[:2]
             im = make_bg(w, h)
             im[top : top + h, left : left + w] = pic

tests/test_fx.py

@@ -440,28 +440,118 @@ def test_make_loopable():
     close_all_clips(locals())
 
 
-def test_margin():
-    clip = BitmapClip([["RRR", "RRR"], ["RRB", "RRB"]], fps=1)
-
-    # Make sure that the default values leave clip unchanged
-    clip1 = margin(clip)
-    assert clip == clip1
-
-    # 1 pixel black margin
-    clip2 = margin(clip, margin_size=1)
-    target = BitmapClip(
-        [["OOOOO", "ORRRO", "ORRRO", "OOOOO"], ["OOOOO", "ORRBO", "ORRBO", "OOOOO"]],
-        fps=1,
+@pytest.mark.parametrize(
+    ("ClipClass"),
+    (ColorClip, BitmapClip),
+    ids=("ColorClip", "BitmapClip"),
+)
+@pytest.mark.parametrize(
+    (
+        "margin_size",
+        "margins",  # [left, right, top, bottom]
+        "color",
+        "expected_result",
+    ),
+    (
+        pytest.param(
+            None,
+            None,
+            None,
+            [["RRR", "RRR"], ["RRR", "RRR"]],
+            id="default arguments",
+        ),
+        pytest.param(
+            1,
+            None,
+            None,
+            [
+                ["OOOOO", "ORRRO", "ORRRO", "OOOOO"],
+                ["OOOOO", "ORRRO", "ORRRO", "OOOOO"],
+            ],
+            id="margin_size=1,color=(0, 0, 0)",
+        ),
+        pytest.param(
+            1,
+            None,
+            (0, 255, 0),
+            [
+                ["GGGGG", "GRRRG", "GRRRG", "GGGGG"],
+                ["GGGGG", "GRRRG", "GRRRG", "GGGGG"],
+            ],
+            id="margin_size=1,color=(0, 255, 0)",
+        ),
+        pytest.param(
+            None,
+            [1, 0, 0, 0],
+            (0, 255, 0),
+            [["GRRR", "GRRR"], ["GRRR", "GRRR"]],
+            id="left=1,color=(0, 255, 0)",
+        ),
+        pytest.param(
+            None,
+            [0, 1, 0, 0],
+            (0, 255, 0),
+            [["RRRG", "RRRG"], ["RRRG", "RRRG"]],
+            id="right=1,color=(0, 255, 0)",
+        ),
+        pytest.param(
+            None,
+            [1, 0, 1, 0],
+            (0, 255, 0),
+            [["GGGG", "GRRR", "GRRR"], ["GGGG", "GRRR", "GRRR"]],
+            id="left=1,top=1,color=(0, 255, 0)",
+        ),
+        pytest.param(
+            None,
+            [0, 1, 1, 1],
+            (0, 255, 0),
+            [["GGGG", "RRRG", "RRRG", "GGGG"], ["GGGG", "RRRG", "RRRG", "GGGG"]],
+            id="right=1,top=1,bottom=1,color=(0, 255, 0)",
+        ),
+        pytest.param(
+            None,
+            [3, 0, 0, 0],
+            (255, 255, 255),
+            [["WWWRRR", "WWWRRR"], ["WWWRRR", "WWWRRR"]],
+            id="left=3,color=(255, 255, 255)",
+        ),
+        pytest.param(
+            None,
+            [0, 0, 0, 4],
+            (255, 255, 255),
+            [
+                ["RRR", "RRR", "WWW", "WWW", "WWW", "WWW"],
+                ["RRR", "RRR", "WWW", "WWW", "WWW", "WWW"],
+            ],
+            id="bottom=4,color=(255, 255, 255)",
+        ),
+    ),
+)
+def test_margin(ClipClass, margin_size, margins, color, expected_result):
+    if ClipClass is BitmapClip:
+        clip = BitmapClip([["RRR", "RRR"], ["RRR", "RRR"]], fps=1)
+    else:
+        clip = ColorClip(color=(255, 0, 0), size=(3, 2), duration=2).with_fps(1)
+
+    # if None, set default argument values
+    if color is None:
+        color = (0, 0, 0)
+
+    if margins is None:
+        margins = [0, 0, 0, 0]
+    left, right, top, bottom = margins
+
+    new_clip = margin(
+        clip,
+        margin_size=margin_size,
+        left=left,
+        right=right,
+        top=top,
+        bottom=bottom,
+        color=color,
     )
-    assert target == clip2
 
-    # 1 pixel green margin
-    clip3 = margin(clip, margin_size=1, color=(0, 255, 0))
-    target = BitmapClip(
-        [["GGGGG", "GRRRG", "GRRRG", "GGGGG"], ["GGGGG", "GRRBG", "GRRBG", "GGGGG"]],
-        fps=1,
-    )
-    assert target == clip3
+    assert new_clip == BitmapClip(expected_result, fps=1)
 
 
 @pytest.mark.parametrize("image_from", ("np.ndarray", "ImageClip"))