Tool to concatenate video files

Question

The function below is written by me in an app that lets you merge 2 or more video clips together, so I used a text box in the GUI, it asks you how many clips you want to merge, and will do it for you accordingly, however, it looks really messy and it is so repetitive.

def merge_vid():

    number_of_videos = box.get()
    if number_of_videos <= "1":
        messagebox.showwarning(title="Warning", message="Please specify how many videos you want to merge.")

    elif number_of_videos == "2":
        clip1 = VideoFileClip(f"{open_location()}.mp4")
        clip2 = VideoFileClip(f"{open_location()}.mp4")
        final_clip = concatenate_videoclips([clip1, clip2])
        final_clip.write_videofile('Final.mp4', codec="libx264")

    elif number_of_videos == "3":
        clip1 = VideoFileClip(f"{open_location()}.mp4")
        clip2 = VideoFileClip(f"{open_location()}.mp4")
        clip3 = VideoFileClip(f"{open_location()}.mp4")
        final_clip = concatenate_videoclips([clip1, clip2, clip3])
        final_clip.write_videofile('Final.mp4', codec="libx264")

    elif number_of_videos == "4":
        clip1 = VideoFileClip(f"{open_location()}.mp4")
        clip2 = VideoFileClip(f"{open_location()}.mp4")
        clip3 = VideoFileClip(f"{open_location()}.mp4")
        clip4 = VideoFileClip(f"{open_location()}.mp4")
        final_clip = concatenate_videoclips([clip1, clip2, clip3, clip4])
        final_clip.write_videofile('Final.mp4', codec="libx264")

Here's the entire code:

import tkinter

from moviepy.editor import *
from tkinter import *
from tkinter import messagebox
from tkinter import ttk
from tkinter import filedialog
import threading
import requests
import os
from tkinter.filedialog import askopenfilename






def space():
    space = Label(text="", bg="black")
    space.pack()

def open_location():

    global filename_splitted
    filename = askopenfilename()
    filename_ext = os.path.splitext(filename)
    filename_splitted = str(filename_ext).split('/')[4].split(".")[0].split(',')[0].split("'")[0]
    filename_ext_split = filename_ext[1][1:]

    if len(filename) > 1 and filename_ext_split == "mp4":
        path_url.config(text=filename_splitted, fg="green")

    else:
        messagebox.showwarning(title="oops", message="No video given!")
        path_url.config(text="Please specify a file (mp4)!", fg="red")

    return filename_splitted


def merge_vid():

    number_of_videos = box.get()
    video_title = title_box.get()
    if number_of_videos <= "1":
        messagebox.showwarning(title="Warning", message="Please specify how many videos you want to merge.")

    elif video_title == "":
        messagebox.showwarning(title="Warning", message="Please give your clip a title..")

    elif number_of_videos == "2":
        clip1 = VideoFileClip(f"{open_location()}.mp4")
        clip2 = VideoFileClip(f"{open_location()}.mp4")
        final_clip = concatenate_videoclips([clip1, clip2])
        final_clip.write_videofile(f'{video_title}.mp4', codec="libx264")

    elif number_of_videos == "3":
        clip1 = VideoFileClip(f"{open_location()}.mp4")
        clip2 = VideoFileClip(f"{open_location()}.mp4")
        clip3 = VideoFileClip(f"{open_location()}.mp4")
        final_clip = concatenate_videoclips([clip1, clip2, clip3])
        final_clip.write_videofile('Final.mp4', codec="libx264")

    elif number_of_videos == "4":
        clip1 = VideoFileClip(f"{open_location()}.mp4")
        clip2 = VideoFileClip(f"{open_location()}.mp4")
        clip3 = VideoFileClip(f"{open_location()}.mp4")
        clip4 = VideoFileClip(f"{open_location()}.mp4")
        final_clip = concatenate_videoclips([clip1, clip2, clip3, clip4])
        final_clip.write_videofile('Final.mp4', codec="libx264")




def put_music():
    # audio = AudioFileClip("Music.mp3")
    # video1 = VideoFileClip("Final.mp4")
    # final = video1.set_audio(audio)
    # final.write_videofile("output.mp4")
    pass



root = Tk()

space()
space()

# root.iconbitmap("yt.ico")
root.title("Video Editor")
root.geometry("350x350")
root.columnconfigure(0, weight=1)
root.config(bg="black")

# space()
# path_btn = Button(root, width=11, height=2, bg="#CC1B25", fg="white", text="Select Video", command=open_location)
# path_btn.pack()

space()
space()
space()
label = Label(root, text="How many videos do you want to merge:", bg="black", fg="white", font=("jost", 9, "bold"))
label.pack()

EntryVar = StringVar()
box = Entry(root, width=7, textvariable=EntryVar)
box.pack()

space()

title = Label(root, text="Name your merged video clip:", bg="black", fg="white", font=("jost", 9, "bold"))
title.pack()

EntryVar = StringVar()
title_box = Entry(root, width=17, textvariable=EntryVar)
title_box.pack()

space()

merge_btn = Button(root, width=27, height=3, bg="green", fg="white", text="Choose and Merge Videos", command=merge_vid)
merge_btn.pack()

space()
space()

# start_merge_btn = Button(root, width=27, height=3, bg="green", fg="white", text="Start Merge", command=start_merge)
# start_merge_btn.pack()

path_url = Label(root, text="", fg="red", bg="black", font=("jost", 9, "bold"))
path_url.pack()






root.mainloop()

Answer 1

TL;DR: To shorten the repetitive code-blocks and reduce duplication, go to 2nd section "Extract method".

Make code readable

To do so we can:

1. improve naming of variables, functions and classes
2. write statements that are self-explaining
3. express intend by telling what we want to achieve and why
4. be clear about expectations and (pre-/post-) conditions

Improve naming

merge_vid() has deserved a better name.

A rule-of-thumb for naming:

• don't use abbreviations or acronyms that are not common
• better spell-out words in correct language to leverage auto-completion and satisfy spellchecking

Write self-explaining lines

Assume the number_of_videos (well describing and honest name!) is entered by the user and given as first input. It's not that clear from box.get() which UI element is used, nor what can be expected to get. Intent of this line is to get a number. So we could name the element textbox_video_number (or somehow incorporate the related label near the box). Also we should convert to an integer, like int(textbox_video_number.get()). Maybe we can also give the user a hint for expected numerical input before. Even better if UI-elements allow only to enter numbers in a specific range, have hints or validation.

Tell your intend like a story

Further it looks like open_location() starts a UI-dialogue which waits for user-response. In the name neither the user-interaction nor the expected return is expressed. The name could tell a story: it asks the user to choose a file and returns the path and base-name. Here we can improve naming to express that intent.

Express your conditions clearly

The guard-statement if number_of_videos <= "1" can also check for a valid range, not only min, but also max. For example, if number_of_videos not in range(2, 5) verifies that a valid number is between min 2 and max 4 (or at least 2 but not more than 4).

Extract method (refactoring)

This can be used to de-duplicate and shorten the code. A code-block that is extracted to a method can be named for his intend, and re-used repetitively.

How to identify blocks to extract?

You already recognized a repetitive pattern in your code and suspect some blocks to be somewhat unnecessarily repeated.

Can you express what the following blocks do for the valid given number? First try to add a comment like a summarizing title on top.

    # ask for given number of video files and concatenate them
    elif number_of_videos == "2":
        clip1 = VideoFileClip(f"{open_location()}.mp4")
        clip2 = VideoFileClip(f"{open_location()}.mp4")
        final_clip = concatenate_videoclips([clip1, clip2])
        final_clip.write_videofile('Final.mp4', codec="libx264")

How to extract?

Now since you gave this block a title and figured out its dependencies, you can define a function as future replacement for this block. Use the title comment as name, e.g.:

def ask_for_videos_and_concatenate(number_of_videos):
    # ask the user for a list of video files (all must have extension mp4)
    video_files = []
    # read the files to clip objects
    clips = []
    # concatenate the list of clips to a single MP4 using specific code
    final_clip = None

    return final_clip

Then fill this dummy or method-stub with code to live. The code comes from your repetitive blocks (which contain duplicated parts depending on the number).

The extracted method and its usage

Finally you might end in:

def ask_for_videos_and_concatenate(number_of_videos):
    # ask the user for a list of video files (all must have extension mp4)
    video_files = []
    for i in range(0, number_of_videos):  # loop exactly n times (0..n-1)
        choosen_file = open_location()  # rename the method to show the UI-aspect: ask the user for a file (path/location)
        video_files.append(f"{choosen_file}.mp4")
    # read the files to clip objects
    clips = [VideoFileClip(f) for f in video_files]  # list-comprehension to shorten
    # concatenate the list of clips to a single MP4 using specific code
    final_clip = concatenate_videoclips( clips )  # now argument clips is a list ;-)
    final_clip.write_videofile('Final.mp4', codec="libx264")

    return final_clip


# and your repetitive blocks have all reduced to a ..
def merge_videos():
    number_of_videos = int(box.get())  # read the user-input as number from text-box
    if number_of_videos not in range(2,5):
        messagebox.showwarning(title="Warning", message="Please specify how many videos you want to merge. Must be at least 2 and maximum 4.")
        return   # fast-fail exit: if invalid number then nothing to merge

    #  .. single line: the happy path
    return ask_for_videos_and_concatenate(number_of_videos)  

Now merge_vids() became a UI-function (reading user-input, validating and fulfilling the action), might be used as callback for a click-event.

Answer 2

space is an interesting antipattern. UI toolkits were not designed for layout to be defined by placeholder elements; don't do this.

Don't use globals.

Use pathlib.

askopenfilename is not being used correctly here. You need to pass it your extension, and set multiple=True. Once this is done, assuming that all of the videos to be merged exist in the same directory (which I think is fairly safe), you can delete your entire user interface. Or if you need to support source files from multiple directories, keep looping over askopenfilename until the user clicks Cancel.

Delete put_music as it's unused.

Add PEP484 typehints.

Suggested

from tkinter.filedialog import askopenfilename, asksaveasfilename
from typing import Iterator

import moviepy.editor as mov


FILE_TYPES = (
    ('MP4 Videos', '*.mp4'),
)


def open_clips() -> Iterator[mov.VideoFileClip]:
    for filename in askopenfilename(
        title='Choose videos to merge',
        filetypes=FILE_TYPES,
        multiple=True,
    ):
        yield mov.VideoFileClip(filename)


def save_video(final_clip: mov.VideoFileClip) -> None:
    dest = asksaveasfilename(
        title='Choose merged output destination',
        filetypes=FILE_TYPES,
        confirmoverwrite=True,
    )
    final_clip.write_videofile(dest, codec='libx264')


def main() -> None:
    clips = tuple(open_clips())
    merged = mov.concatenate_videoclips(clips)
    save_video(merged)


if __name__ == '__main__':
    main()