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.
1 Answer 1
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.
-
\$\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\$AlexanderKS27– AlexanderKS272023年10月16日 17:41:45 +00:00Commented 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\$AlexanderKS27– AlexanderKS272023年10月16日 17:42:51 +00:00Commented Oct 16, 2023 at 17:42