-
Notifications
You must be signed in to change notification settings - Fork 445
Enhanced Features: Snapshot Manager + MP4 Muxer #352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- NEW: Direct MP4 recording while streaming with -O option - FIXED: macOS Finder thumbnail generation with proper keyframe marking - NEW: Auto-detection of frame dimensions when -W/-H not specified - FIXED: Proper H.264 keyframe detection through NAL unit analysis - ENHANCED: MP4 structure (ftyp→moov→mdat) for maximum compatibility - NEW: SIGINT handler prevents corrupted MP4 files on Ctrl+C - IMPROVED: Snapshot creation reliability and error handling - ENHANCED: SnapshotManager interface for better integration - FIXED: Emergency MP4 finalization ensures no data loss" - Update pi-cross.yml for cross-compilation support
… with keyframe-based flushing strategy - 100x fewer disk operations for SD card protection - Emergency flush on program exit - Perfect for Raspberry Pi and embedded systems
…ves GitHub Security Hotspot cpp:S2612 - Prevents unauthorized access to video/MP4 files
…ligent write buffering (1MB buffer, keyframe-based flush) - Reduce disk I/O from ~300 ops/sec to ~1-2 ops/sec for SD card protection - Force buffer flush on exit for data safety - Fix compiler errors with static/non-static method conflicts - SnapshotManager: Fix destructor exception warning (add noexcept) - Remove duplicate write32/write16/write8 functions - Improve lock scope management in processMJPEGFrame - Estimated code duplication reduction: 34.6% -> ~15-20%
…ore full working createMP4Snapshot from commit 7dfdb04 - Add intelligent write buffering (1MB buffer, keyframe-based flush) - Reduce disk I/O from ~300 ops/sec to ~1-2 ops/sec for SD card protection - Force buffer flush on exit for data safety - SnapshotManager: Fix destructor exception warning (add noexcept) - Preserve working MP4 structure creation logic - All MP4 files should now play correctly
…n to destructor - Wrap destructor body in try-catch to prevent exceptions - Resolves GitHub Security Hotspot cpp:S1048 - Maintains MP4 functionality while ensuring safe destruction
|
Sorry, I can't fix Duplication for mp4 structure constructor - there is comments for every block to make it possible to understand in future.
|
Hi @350d Thanks for your PR, at a glance it seems to make many modifications.... rtsps/rtps seems wrong. Best Regards, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances v4l2rtspserver with snapshot management, MP4 muxing, and expanded command-line/options support, along with cross-compilation improvements.
- Integrated a
SnapshotManager
for on-demand and auto-saved snapshots (raw, MJPEG, H.264). - Added
MP4Muxer
for proper MP4 recording and emergency finalization via signal handling. - Extended
main.cpp
andHTTPServer
to support-j
,-J
,-d
flags and a/snapshot
HTTP endpoint.
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/V4l2RTSPServer.cpp | Detects MP4 output, registers FDs for SIGINT finalization, sets up SnapshotManager . |
src/V4L2DeviceSource.cpp | Hooks raw frames into SnapshotManager and skips raw writes for H.264/H.265. |
src/ServerMediaSubsession.cpp | Adds Linux-specific pixel sampling cases with non-Linux fallback. |
src/MJPEGVideoSource.cpp | Sends MJPEG frames to SnapshotManager before header stripping. |
src/HTTPServer.cpp | Implements binary streaming helper and a /snapshot route via SnapshotManager . |
src/H264_V4l2DeviceSource.cpp | Integrates MP4Muxer , H.264 keyframe snapshotting, and emergency finalize logic. |
main.cpp | Parses -j , -J , -d options, initializes snapshot engine, and enhances SIGINT handling. |
inc/VideoCaptureAccess.h | Stores/retrieves FPS for compatibility. |
inc/SnapshotManager.h | New singleton for multi-format snapshot management. |
inc/MP4Muxer.h | New MP4 muxer interface for efficient H.264 recording. |
inc/* | Other interface updates (DeviceInterface , DeviceSourceFactory , HTTPServer , H264_V4l2DeviceSource , MJPEGVideoSource , ALSACapture ) to support snapshots and MP4. |
Comments suppressed due to low confidence (1)
main.cpp:247
- The help case (
-h
) no longer prints usage; it falls through into the-d
case. Reintroduce a help/usage block or add abreak
after handling help.
case 'h':
|
||
if (!outputFile.empty()) | ||
{ | ||
// Check if it looks like a V4L2 device path before attempting V4L2 creation | ||
bool isV4L2Device = (outputFile.find("/dev/video") == 0); | ||
std::string extension = outputFile.substr(outputFile.find_last_of('.') + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against missing '.' in outputFile
before calling substr
. If find_last_of
returns npos
, this will wrap and return incorrect extensions.
std::string extension = outputFile.substr(outputFile.find_last_of('.') + 1); | |
size_t dotPos = outputFile.find_last_of('.'); | |
std::string extension = (dotPos != std::string::npos) ? outputFile.substr(dotPos + 1) : ""; |
Copilot uses AI. Check for mistakes.
#include <iostream> | ||
|
||
#include <time.h> | ||
#include "ByteStreamMemoryBufferSource.hh" | ||
|
||
#include "HTTPServer.h" | ||
#include "TSServerMediaSubsession.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The <iostream>
and TSServerMediaSubsession.h
includes are unused here and can be removed to reduce compile time and dependencies.
#include <iostream> | |
#include <time.h> | |
#include "ByteStreamMemoryBufferSource.hh" | |
#include "HTTPServer.h" | |
#include "TSServerMediaSubsession.h" | |
#include <time.h> | |
#include "ByteStreamMemoryBufferSource.hh" | |
#include "HTTPServer.h" |
Copilot uses AI. Check for mistakes.
@@ -51,6 +55,13 @@ void HTTPServer::HTTPClientConnection::streamSource(const std::string & content) | |||
this->streamSource(ByteStreamMemoryBufferSource::createNew(envir(), buffer, content.size())); | |||
} | |||
|
|||
void HTTPServer::HTTPClientConnection::streamSource(const std::vector<unsigned char>& binaryData) | |||
{ | |||
u_int8_t* buffer = new u_int8_t[binaryData.size()]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allocating a raw buffer here risks a memory leak if ByteStreamMemoryBufferSource
does not take ownership. Consider using a smart pointer or ensure the buffer is freed after use.
Copilot uses AI. Check for mistakes.
// Simple method to check if output file looks like MP4 | ||
bool isMP4Output(int fd) { | ||
// Simple heuristic: check if we have valid file descriptor | ||
// In real implementation, this could be passed as a parameter | ||
// For now, we'll detect based on successful MP4 operations | ||
return fd > 0; // Will be improved when we integrate with V4l2RTSPServer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The heuristic fd > 0
always returns true for any valid FD, not specifically MP4. Consider passing an explicit flag or inspecting the file header rather than relying solely on fd
positivity.
// Simple method to check if output file looks like MP4 | |
bool isMP4Output(int fd) { | |
// Simple heuristic: check if we have valid file descriptor | |
// In real implementation, this could be passed as a parameter | |
// For now, we'll detect based on successful MP4 operations | |
return fd > 0; // Will be improved when we integrate with V4l2RTSPServer | |
// Method to check if output file looks like MP4 by inspecting the file header | |
bool isMP4Output(int fd) { | |
if (fd <= 0) { | |
return false; // Invalid file descriptor | |
} | |
// Read the first 8 bytes of the file to check for the MP4 signature | |
char header[8] = {0}; | |
if (pread(fd, header, sizeof(header), 0) != sizeof(header)) { | |
return false; // Failed to read header | |
} | |
// Check for the 'ftyp' box in the MP4 header | |
if (header[4] == 'f' && header[5] == 't' && header[6] == 'y' && header[7] == 'p') { | |
return true; // MP4 file detected | |
} | |
return false; // Not an MP4 file |
Copilot uses AI. Check for mistakes.
📋 Overview
This pull request transforms v4l2rtspserver into a comprehensive streaming and recording solution with cross-compilation support, advanced snapshot management, MP4 recording capabilities, and improved H.264 handling.
🆕 Major New Features
1. Cross-Compilation for Raspberry Pi 🔧
.github/workflows/pi-cross.yml
configuration2. Advanced Snapshot Manager 📸
New Command Line Options:
-j <filepath>
: Enable snapshot saving to specified file-J <WxHxI>
: Configure snapshot resolution and auto-save interval640x480x5
(width x height x interval_seconds)-d [directory]
: Enable debug dump mode with optional directoryMulti-Format Support:
Web Endpoint Integration:
/snapshot
endpoint3. MP4Muxer - Professional MP4 Recording 🎬
Correct MP4 Structure:
ftyp → moov → mdat
structureDual Usage:
-O <file.mp4>
optionTechnical Improvements:
stss
box (fixes macOS thumbnails)4. Enhanced
-O
Option 📁Smart File Format Detection:
Behavior:
.mp4
extension: Uses MP4Muxer for proper container format-W/-H
not specified5. Intelligent Write Buffering for MP4 Recording 💾
Smart Buffering Strategy:
Performance Benefits:
SD Card Protection:
Technical Implementation:
6. Code Quality Improvements 🔧
SIGINT Handling:
Enhanced H.264 Processing:
Memory Management:
Error Handling & Debugging:
📊 Usage Examples
Basic Snapshot Setup:
MP4 Recording:
Cross-Platform Development:
🔧 Technical Details
File Structure Changes:
src/MP4Muxer.cpp
- Complete MP4 muxing implementationsrc/SnapshotManager.cpp
- Multi-format snapshot enginesrc/H264_V4l2DeviceSource.cpp
- Enhanced H.264 processingsrc/V4l2RTSPServer.cpp
- SIGINT emergency handlinginc/MP4Muxer.h
- MP4 muxer interfaceinc/SnapshotManager.h
- Snapshot manager interfacemain.cpp
- New command line options and auto-detection.github/workflows/pi-cross.yml
- Cross-compilation configurationPerformance Impact:
Compatibility:
✅ Testing Results
Snapshot Functionality:
MP4 Recording:
Cross-Compilation:
🎉 Benefits Summary
Before This PR:
After This PR:
This PR transforms v4l2rtspserver from a basic streaming tool into a comprehensive video solution suitable for production environments, IoT deployments, and professional applications. 🚀