feature/new-recording-fileformat #223
Conversation
…try timeout condition
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the video file metadata handling and modernizes API headers for cloud uploads. The changes refactor how file information (FPS, resolution, duration) is embedded in filenames and introduce new HTTP headers for Vault uploads while maintaining backward compatibility.
- Adds FPS and resolution to recording filenames for better metadata tracking
- Introduces new X-Hub, X-Vault, and X-Agent HTTP headers for cloud uploads
- Refactors if-else chains to switch statements for better code maintainability
- Introduces new models package for structured media metadata
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| machinery/src/cloud/Vault.go | Adds new HTTP headers for Hub, Vault, and Agent information; fixes retry timeout comparison; removes Content-Type header for first upload |
| machinery/src/cloud/Hub.go | Adds blank lines for improved code readability |
| machinery/src/cloud/Cloud.go | Refactors if-else chain to switch statement and adds comments about filename structure |
| machinery/src/capture/main.go | Extracts camera metadata (FPS, resolution), adds new models package import, updates filename format to include FPS and resolution, creates AgentMedia objects |
Comments suppressed due to low confidence (4)
machinery/src/cloud/Vault.go:94
- The X-Agent-File-Duration header is set to 'mp4' (a format) instead of a duration value. This should contain the actual duration of the video file.
machinery/src/cloud/Vault.go:92 - The FPS and resolution values are hardcoded ('15' and '640x480'), but the capture/main.go file shows these values are extracted from the actual camera metadata. These headers should use the actual video file metadata instead of hardcoded values.
machinery/src/cloud/Vault.go:193 - Same issue as the primary storage upload: FPS and resolution values are hardcoded instead of using actual video file metadata. Additionally, the X-Agent-File-Duration header is missing entirely for the secondary storage upload, creating inconsistency between primary and secondary uploads.
machinery/src/capture/main.go:189 - This definition of recordingName is never used.
recordingName := models.NewAgentMedia(
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
machinery/src/capture/main.go
Outdated
| recordingName := models.NewAgentMedia( | ||
| models.WithName(s), | ||
| models.WithDuration(duration), | ||
| models.WithStartTime(startRecording), | ||
| models.WithFPS(fps), | ||
| models.WithResolution(cameraResolution), | ||
| ) |
There was a problem hiding this comment.
The recordingName variable is created but never used. The subsequent code still builds the filename string manually using the same 's' variable that was passed to WithName. This creates unused code and suggests incomplete refactoring.
machinery/src/capture/main.go
Outdated
| startRecordingMilliseconds := startRecording % 1000 // convert to milliseconds | ||
|
|
||
| recordingName := models.NewAgentMedia( | ||
| models.WithName(s), |
There was a problem hiding this comment.
The variable 's' is used before it's defined. The NewAgentMedia call on line 189 uses 's' on line 190, but 's' is not defined until line 197. This will cause a compilation error.
Upgraded multiple dependencies to newer versions in go.mod and go.sum, including mp4ff, gortsplib, carbon, gin, webrtc, and others. Added new dependencies such as uug-ai/models and quic-go, and updated Go version to 1.24.9 for improved compatibility and security.
Description
Pull Request: feature/new-recording-fileformat
Motivation and Context
The primary motivation behind this change is to enhance the recording file format used by the Kerberos agent. By updating dependencies and incorporating additional metadata into the recording files, we aim to improve the functionality, interoperability, and user experience of the system. This update will provide more comprehensive information about the recordings, such as camera resolution, frame rate, and duration, which will be useful for both internal processing and external integrations.
Changes Overview
Dependency Updates:
machinery/go.modto their latest versions to ensure better performance, security, and compatibility.Recording Metadata Enhancements:
Cloud Upload Adjustments:
HandleUpload,UploadKerberosHub, andUploadKerberosVault) to include new headers that carry the additional metadata.Code Refactoring:
modelsOldto distinguish between old and new model structures.Benefits to the Project
Enhanced Recordings:
The new recording file format provides richer metadata, making it easier to understand and utilize the recordings for various applications, including analysis and archiving.
Improved Cloud Integration:
By standardizing the metadata headers across different cloud providers, we ensure that the recordings are consistently uploaded with all relevant information, facilitating better cloud management and retrieval.
Dependency Management:
Keeping dependencies up-to-date ensures that the project leverages the latest features and security patches, leading to a more robust and reliable system.
Code Quality:
Refactoring and organizing the code improve the overall quality, making it easier to maintain and extend in the future.
This pull request is a significant step forward in enhancing the Kerberos agent's recording capabilities and overall system performance.