3
\$\begingroup\$

I’d love some critique, pointers or any improvement to my code. My project was mainly to record a video, through a Flask Application (online), then save it and display it on the Flask Application. Will probably do it through Vimeo API or something.

It’s just a demo. I still need to remove the timer, so it records until someone clicks stop recording. (But this is being used at the moment to save on storage on my Raspberry Pi) Then I will have to write some new code so that the files don’t overwrite each other. Potentially use datetime or a while loop counter.

I am using SSH to control my Raspberry Pi through VSC. The camera is a C920 [HD Pro Webcam C920] that is plugged in with a USB to Raspberry Pi. I tried recording both video and audio at the same time, but I ran into trouble doing that, that’s why I decided to split it into recording separately and combining it later.

One potential issue I have noticed is that depending on which USB port I use it can switch the audio output, but that can occur randomly as well, often when a reboot has occurred. My solution might be to disable the other audio outputs, but I don’t know yet. But this again hasn’t been an issue lately.

The current code is this:

import subprocess
import os
import time

# Directory of files saved
output_directory = "/home/pi/Videos"

# Start recording
def start_recording():
    # Filenames for video and audio
    video_filename = os.path.join(output_directory, "video.mp4")
    audio_filename = os.path.join(output_directory, "audio.wav")

    # Start record video
    start_video = [
        "ffmpeg",
        "-thread_queue_size", "512",
        "-f", "v4l2",
        "-input_format", "mjpeg",
        "-video_size", "1280x720",
        "-framerate", "10",
        "-i", "/dev/video0",
        "-c:v", "libx264",
        "-maxrate", "1000k",
        "-bufsize", "128k",
        "-y",
        video_filename
    ]

    # Start record audio
    start_audio = [
        "ffmpeg",
        "-f", "alsa",
        "-acodec", "pcm_s16le",
        "-ac", "2",
        "-ar", "44100",
        "-i", "hw:3,0", # Can sometimes change to hw:1,0
        "-y",
        audio_filename
    ]

    # Execute video and audio recording commands in the background
    video_process = subprocess.Popen(start_video)
    audio_process = subprocess.Popen(start_audio, stderr=subprocess.PIPE)

    return video_process, audio_process, video_filename, audio_filename

# Stop recording
def stop_recording(video_process, audio_process, video_filename, audio_filename):
    # Terminate video recording process
    video_process.terminate()

    # Terminate audio recording process
    audio_process.terminate()

    # Wait for the processes to finish
    video_process.wait()
    audio_process.wait()

    # Combine video and audio
    output_filename = os.path.join(output_directory, "output.mp4")
    combine_command = [
        "ffmpeg",
        "-i", video_filename,
        "-i", audio_filename,
        "-c:v", "copy",
        "-c:a", "aac",
        "-strict", "-2",
        "-y",
        output_filename
    ]

    subprocess.run(combine_command)

    print("Recording stopped and files merged.")

# Start recording when needed (when a button is pressed)
video_process, audio_process, video_filename, audio_filename = start_recording()

# Sleep for a while (simulating the recording duration)
# (For now demo reasons. will be removed)
time.sleep(10)  # Record for 10 seconds; adjust as needed 

# Stop recording when needed (when a button is pressed)
stop_recording(video_process, audio_process, video_filename, audio_filename)

The code is still a WIP as I still need to code that the video itself needs to be uploaded to a site like Vimeo, and then make it to embed it into my Flask-application. There will be start and end record button on said Flask-application.

\$\endgroup\$

1 Answer 1

Reset to default
3
\$\begingroup\$

use Path

output_directory = "/home/pi/Videos"

Prefer to assign ... = Path("~pi/Videos").expanduser(). That way you can compactly refer to output_directory / "audio.wav" without need of a .join().

document the return values

def start_recording():
    ...
    return video_process, audio_process, video_filename, audio_filename

When we're hacking on a personal project, often there is little need to document tons of details. But here, the return type is complex enough that type annotation would offer benefits, both for human readers and for mypy checking:

def start_recording() -> tuple[Popen[bytes], Popen[bytes], str, str]:

And tacking on a one-sentence """docstring""" wouldn't hurt.

That collection of four related variables is almost big enough that it starts to look ungainly. Consider putting this code in a class so they become self. attributes. Or perhaps a @dataclass. Or a pair of dataclass objects, each holding a Popen and a str.

examine the exit status

    subprocess.run(combine_command)

We anticipate that this ffmpeg command typically succeeds.

Usually we express such a belief in this way:

    subprocess.check_call(combine_command)

The check_call will raise upon child failure, conveniently showing us a helpful diagnostic message.

progress bar

time.sleep(10)

Or equivalently, assign n = 10 and then it's sleep(n).

For such a brief duration there's little room for confusion. But when you start using larger n values, you might want some feedback on whether we're nearly done, using the convenient tqdm library.

from tqdm import tqdm
...
n = 120
for delay in tqdm([1] * n):
    sleep(delay)

If you wish to offer CLI access to this code, consider using the typer library.

The code relies on coupling to some module level / global variables. It would benefit from pushing functions down into class methods.

Working out how to capture a combined A/V stream, to avoid synchronization trouble, seems the chief technical challenge to solve. That's outside the scope of this PR.

This WIP appears to achieve its current goals.

I would be willing to delegate or accept maintenance tasks on this codebase.

\$\endgroup\$
2
  • \$\begingroup\$ Wow, thanks for your great feedback. I really appreciate it. It really helps me becoming a better coder. But sure, you can maintain and/or delegate any tasks you’d like on this codebase:) I don’t know if it is worth mentioning it, but I do believe it’s the hardware capabilities of the raspberry pie that doesn’t allow for simultaneously recording of both audio and video. The raspberry pi is a 4 – model b (2GB) and the web camera is actually a C920 [HD Pro Webcam C920] USB (thought it was a Logitech because I saw a Logi watermark on it). \$\endgroup\$ Commented Oct 16, 2023 at 17:41
  • \$\begingroup\$ I should mention I tried a lot of different command prompts through SSH with FFmpeg. I ended up with this prompt “ffmpeg -f v4l2 -input_format mjpeg -video_size 1280x720 -framerate 10 -i /dev/video0 -f alsa -acodec pcm_s16le -ac 2 -i hw:1,0 -c:v libx264 -maxrate 1000k -bufsize 1024k-c:a aac -strict -2 -y /home/pi/Videos/output_with_audio_mjpeg.mp4” and it shouldn’t wouldn’t allow me to record with audio, and I tried both of the different formats. I would've later made a python script to do this action, but I needed to know if it was worth the effort \$\endgroup\$ Commented Oct 16, 2023 at 17:42

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.