Feature/labimp 7672 - #42
Conversation
- Created a new script to test the upload of keyframe preannotations. - added method for pre-annotation upload to video project - modify SDK notebook accordingly
- Created a new script `test_preannotation_api.py` to test the upload of keyframe preannotations. - added method for pre-annotation upload to video project - modify SDK notebook accordingly
- Created a new script to test the upload of keyframe preannotations. - added method for pre-annotation upload to video project - modify SDK notebook accordingly
- Created a new script `test_preannotation_api.py` to test the upload of keyframe preannotations. - added method for pre-annotation upload to video project - modify SDK notebook accordingly
…into LABIMP-7672
- minor changes to SDK
- Add keyframe annotation upload to Notebook - added coco to video json converter
…ature/LABIMP-7672
Code Review SummaryOverviewThis PR introduces video keyframe extraction and scene detection functionality with three detection algorithms (FFMPEG, SSIM, PySceneDetect), video preannotation upload capabilities, and supporting utilities. Stats: +2,441 additions, -872 deletions across 16 files 🔴 Critical Issues1. Hardcoded Local File Paths (Security & Portability)Multiple files contain hardcoded Windows-specific paths that will fail in production:
Impact: Code will fail for any user without this exact directory structure. This is a deployment blocker. 2. BASE_URL Changed to Non-Production (Production Issue)
Impact: This will route all production traffic to a QA/staging environment. This must be reverted or made configurable. 3. Test File in Production Code (Code Organization)
Impact: Test files should be in
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -1,4 +1,4 @@ | |||
| BASE_URL = "https://api.labellerr.com" | |||
| BASE_URL = "https://api-gateway-qcb3iv2gaa-uc.a.run.app" | |||
There was a problem hiding this comment.
🔴 Critical: Production URL Changed to QA Environment
This changes the base URL from production (https://api.labellerr.com) to what appears to be a QA/staging environment.
Issues:
- All SDK users will hit the QA environment instead of production
- This breaks existing production integrations
- No environment-based configuration
Recommendation:
import os
BASE_URL = os.getenv("LABELLERR_BASE_URL", "https://api.labellerr.com")Or use a proper config system that allows override for testing while defaulting to production.
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| video_path = r"D:\Professional\Labellerr_SDK\SDKPython\labellerr\notebooks\Labellerr_datasets\354681d3-034a-4d66-b070-365f4bd11d8a\2a8d96ca-9161-4dee-ad3b-a5faf301bc6c.mp4" |
There was a problem hiding this comment.
🔴 Critical: Hardcoded Local Path
This hardcoded Windows path will fail for anyone else using this code. Remove this __main__ block or use a configurable path.
Better approach:
if __name__ == "__main__":
import sys
if len(sys.argv) < 2:
print("Usage: python ffmpeg_detect.py <video_path>")
sys.exit(1)
video_path = sys.argv[1]
# Rest of code...
|
|
||
| if __name__ == "__main__": | ||
| # Example usage | ||
| video_path = r"D:\Professional\Labellerr_SDK\SDKPython\labellerr\notebooks\Labellerr_datasets\354681d3-034a-4d66-b070-365f4bd11d8a\2a8d96ca-9161-4dee-ad3b-a5faf301bc6c.mp4" |
There was a problem hiding this comment.
🔴 Critical: Hardcoded Local Path
Same issue as in ffmpeg_detect.py - this hardcoded Windows path should be removed or parameterized.
| import os | ||
|
|
||
| from dotenv import load_dotenv | ||
|
|
||
| from labellerr.client import LabellerrClient | ||
| from labellerr.core.projects.video_project import LabellerrProject | ||
|
|
||
| # Load environment variables from .env file | ||
| load_dotenv(r"D:\Professional\Labellerr_SDK\dev.env") |
There was a problem hiding this comment.
🔴 Critical: Test File in Wrong Location + Hardcoded Path
Issues:
- Test files should be in
tests/directory, not in source code - Hardcoded Windows-specific path:
r"D:\Professional\Labellerr_SDK\dev.env" - This will be packaged with the SDK unnecessarily
Action:
- Move to
tests/integration/test_video_preannotation.py - Use environment variables or pytest fixtures for configuration
- Remove hardcoded paths
| os.cpu_count() or 1, # Number of CPU cores (default to 1 if None) | ||
| len(batches), # Number of batches | ||
| 20, | ||
| 5, |
There was a problem hiding this comment.
Changed from 20 to 5 workers, which will significantly slow down parallel uploads.
Questions:
- What issue was this change addressing?
- Was there a race condition or resource exhaustion?
- Should this be configurable by users?
Recommendation:
# Allow configuration via environment variable
DEFAULT_MAX_WORKERS = int(os.getenv("LABELLERR_MAX_WORKERS", "20"))
max_workers = min(
os.cpu_count() or 1,
len(batches),
DEFAULT_MAX_WORKERS,
)Please add a comment explaining the rationale for this change.
| from .image_project import ImageProject as LabellerrImageProject | ||
| from .video_project import VideoProject as LabellerrVideoProject | ||
| from .text_project import TextProject as LabellerrTextProject | ||
| from .base import LabellerrProject |
There was a problem hiding this comment.
Lines 18-19 duplicate imports from earlier in the file:
LabellerrProjectimported on line 14LabellerrAnnotationTemplateimported on line 10
Remove the duplicates on lines 18-19.
| except Exception as e: | ||
| raise LabellerrError(f"Failed to delete key frames: {str(e)}") | ||
|
|
||
| def upload_keyframe_preannotations(self, video_json_file_path: str = None) -> Any: |
There was a problem hiding this comment.
📝 Documentation Issue: Mentions Non-existent Parameters
The docstring mentions deprecated parameters (annotation_format, annotation_file, conf_bucket, _async) that don't exist in the function signature.
Fix:
def upload_keyframe_preannotations(self, video_json_file_path: str) -> Any:
\"\"\"
Uploads pre-annotations for video project in video_json format.
:param video_json_file_path: Path to the video JSON file containing pre-annotations
:return: Response from the API
:raises LabellerrError: If file path is invalid or upload fails
\"\"\"
| from typing import Any, Dict, List, Optional, Union | ||
|
|
||
| # Try to import detectors (optional dependencies) | ||
| try: |
There was a problem hiding this comment.
♻️ Code Duplication: Validation Logic
All three detector classes (FFMPEGSceneDetect, SSIMSceneDetect, PySceneDetect) have identical:
_validate_video_file()methodSUPPORTED_EXTENSIONSconstant
Recommendation: Extract to a shared base class or utility module:
class BaseVideoDetector(Singleton):
SUPPORTED_EXTENSIONS = {".mp4", ".avi", ".mov", ...}
def _validate_video_file(self, video_path: str) -> None:
# Shared implementation
...This would reduce duplication and ensure consistent validation across all detectors.
|
|
||
| def download_create_video_auto_cleanup( | ||
| self, output_folder: str = "./Labellerr_datastets" | ||
| self, output_folder: str = "./Labellerr_datasets" |
There was a problem hiding this comment.
✅ Good Fix: Typo Corrected
Nice catch fixing the typo from "datastets" to "datasets" in the default parameter!
| check=True, | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=30, # 30 second timeout per frame |
There was a problem hiding this comment.
🔧 Magic Number: Hardcoded Timeout
30-second timeout should be a configurable constant or parameter, especially for large videos or slow systems.
Suggestion:
FRAME_EXTRACTION_TIMEOUT = 30 # seconds
# In method:
timeout=self.FRAME_EXTRACTION_TIMEOUT
| } | ||
|
|
||
|
|
||
| def coco_to_video_json( |
There was a problem hiding this comment.
🧪 Missing Test Coverage
This is a complex function with multiple edge cases (filename parsing, format conversion, grouping logic). It needs comprehensive unit tests covering:
- Valid COCO JSON with bbox annotations
- Valid COCO JSON with polygon/segmentation annotations
- Invalid filename formats
- Multiple annotations per frame
- Multiple frames per video
- Edge case: Empty annotations list
- Edge case: Missing required fields
Consider adding tests in tests/unit/test_video_sampling.py
| raise VideoFileError(f"Video file is not readable: {video_path}") | ||
|
|
||
| def detect_and_extract( | ||
| self, video_path: str, threshold: float = 0.3, resize_dim: tuple = (320, 240) |
There was a problem hiding this comment.
🔧 Magic Numbers: Hardcoded Parameters
Default threshold (0.3) and resize dimensions (320, 240) should be named constants at class level:
class SSIMSceneDetect(Singleton):
SUPPORTED_EXTENSIONS = {...}
DEFAULT_THRESHOLD = 0.3
DEFAULT_RESIZE_DIM = (320, 240)
def detect_and_extract(
self,
video_path: str,
threshold: float = DEFAULT_THRESHOLD,
resize_dim: tuple = DEFAULT_RESIZE_DIM
):This improves maintainability and makes it clear these are configurable defaults.
| ] | ||
|
|
||
| if not video_files: | ||
| print(f"⚠️ No video files found in {dataset_dir}") |
There was a problem hiding this comment.
📝 Consider Logging Instead of Print
For a production SDK, consider using Python's logging module instead of print() statements. This allows users to control verbosity and integrate with their logging systems:
import logging
logger = logging.getLogger(__name__)
# Instead of print():
logger.info(f"Found {len(video_files)} video files to process")
logger.warning(f"⚠️ No video files found in {dataset_dir}")
logger.error(f"✗ Failed: {str(e)}")Users can then configure logging level as needed.
| try: | ||
| # Use file_id as folder name | ||
| folder_name = self.file_id | ||
| # Use [Dataset_id]+[File_id]+[File_name] as folder name |
There was a problem hiding this comment.
✅ Good Improvement: Better Folder Naming
Good enhancement to use descriptive folder names with dataset+file+name structure. This makes it much easier to identify videos in the file system.
Minor suggestion: Consider documenting this naming convention in the class docstring or module documentation for users.
| video_filename = f"{self.dataset_id}+{self.file_id}+{self.file_name}" | ||
| elif self.dataset_id: | ||
| video_filename = f"{self.dataset_id}+{self.file_id}.mp4" | ||
| else: |
There was a problem hiding this comment.
🐛 Potential Bug: String Formatting Error
Line 316 has incorrect string literal - the closing brace should be outside the string:
# Current (incorrect):
print("{'='*60}\n") # This prints: {'='*60}
# Should be:
print(f"{'='*60}\n") # This prints: ====================...Or simply:
print(f"{'='*60}\n")
Code Review SummaryI've completed a comprehensive review of this PR. Below are the key findings organized by category: 🔴 Critical Issues
🟡 Code Quality Issues
🟢 Security Concerns
📊 Performance Considerations
✅ Test Coverage
📝 Additional Notes
I've added inline comments on specific lines that need attention. Please address the critical issues before merging. |
| @@ -1,4 +1,4 @@ | |||
| BASE_URL = "https://api.labellerr.com" | |||
| BASE_URL = "https://api-gateway-qcb3iv2gaa-uc.a.run.app" | |||
There was a problem hiding this comment.
🔴 Critical: Hardcoded QA Environment URL
This changes the BASE_URL to a QA environment in production code. This should:
- Be reverted to the production URL
- Use environment variables for different environments
- Have proper configuration management
| BASE_URL = "https://api-gateway-qcb3iv2gaa-uc.a.run.app" | |
| BASE_URL = os.getenv("LABELLERR_BASE_URL", "https://api.labellerr.com") |
| # - select filter: Only pass through I-frames (PICT_TYPE_I) | ||
| # - showinfo: Print detailed information about each frame to stderr | ||
| # - null output: Don't actually save frames, just analyze | ||
| command = [ |
There was a problem hiding this comment.
🟡 Security: Command Injection Risk
The subprocess call doesn't sanitize the video_path input. If the path contains special characters or is user-controlled, this could lead to command injection.
Recommendation:
# Validate and sanitize the path first
video_path = os.path.abspath(video_path)
if not video_path.startswith(expected_base_path):
raise VideoFileError("Invalid video path")
| # Using actual frame numbers ensures frames are named correctly | ||
| # (e.g., frame 250 from video → video_name+frame_250.jpg) | ||
| selected_frames = [] | ||
| for idx, frame_num in enumerate(frame_numbers, 1): |
There was a problem hiding this comment.
Each frame is extracted with a separate FFMPEG call. For videos with many I-frames, this is extremely inefficient.
Consider:
- Extracting all frames in a single FFMPEG command using the
selectfilter - Batch processing frames in groups
- Using frame extraction with output pattern like
frame_%04d.jpg
This could reduce extraction time by 10-100x for videos with many keyframes.
| check=True, | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=30, # 30 second timeout per frame |
There was a problem hiding this comment.
🟡 Code Quality: Magic Number
The 30-second timeout is hardcoded. This should be:
- A class constant
- Configurable per video size/complexity
- Documented why 30 seconds was chosen
FRAME_EXTRACTION_TIMEOUT = 30 # Class constant
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| video_path = r"D:\Professional\Labellerr_SDK\SDKPython\labellerr\notebooks\Labellerr_datasets\354681d3-034a-4d66-b070-365f4bd11d8a\2a8d96ca-9161-4dee-ad3b-a5faf301bc6c.mp4" |
There was a problem hiding this comment.
🔴 Critical: Hardcoded Test Path
This hardcoded Windows-specific path should not be in the repository. Remove the __main__ block or use a configurable path.
| video_path = r"D:\Professional\Labellerr_SDK\SDKPython\labellerr\notebooks\Labellerr_datasets\354681d3-034a-4d66-b070-365f4bd11d8a\2a8d96ca-9161-4dee-ad3b-a5faf301bc6c.mp4" | |
| if __name__ == "__main__": | |
| import sys | |
| if len(sys.argv) < 2: | |
| print("Usage: python ffmpeg_detect.py <video_path>") | |
| sys.exit(1) | |
| video_path = sys.argv[1] |
| # ================================================================ | ||
| # PHASE 3: Process remaining frames with SSIM detection | ||
| # ================================================================ | ||
| while True: |
There was a problem hiding this comment.
🟡 Performance: Memory Management
The entire video is processed frame-by-frame without memory management. For large videos, this could cause memory issues.
Consider:
- Processing in batches with periodic garbage collection
- Limiting frame buffer size
- Adding progress callbacks for long-running operations
| # ============================================================================ | ||
|
|
||
|
|
||
| def _extract_video_name_and_frame(filename: str) -> tuple[str, int, int]: |
There was a problem hiding this comment.
🟡 Code Quality: Repeated Logic
The folder naming logic {dataset_id}+{file_id}+{video_name}+FPS{fps} is repeated across multiple files (video_file.py, init.py, etc.). This should be extracted to a utility function:
def generate_video_folder_name(dataset_id: str, file_id: str, file_name: str, fps: Optional[int] = None) -> str:
\"\"\"Generate standardized folder name for video files.\"\"\"
base_name = os.path.splitext(file_name)[0]
parts = [dataset_id, file_id, base_name]
if fps:
parts.append(f"FPS{fps}")
return "+".join(parts)
| # ============================================================================ | ||
|
|
||
|
|
||
| def _extract_video_name_and_frame(filename: str) -> tuple[str, int, int]: |
There was a problem hiding this comment.
The return type tuple[str, int, int] uses Python 3.9+ syntax. For compatibility with Python 3.8, use:
| def _extract_video_name_and_frame(filename: str) -> tuple[str, int, int]: | |
| def _extract_video_name_and_frame(filename: str) -> Tuple[str, int, int]: |
And add import: from typing import Tuple
| except Exception as e: | ||
| raise LabellerrError(f"Failed to delete key frames: {str(e)}") | ||
|
|
||
| def upload_keyframe_preannotations(self, video_json_file_path: str = None) -> Any: |
There was a problem hiding this comment.
🟡 Code Quality: Misleading Docstring
The docstring mentions deprecated parameters (annotation_format, annotation_file, conf_bucket, _async) that don't exist in the function signature. This is confusing.
| def upload_keyframe_preannotations(self, video_json_file_path: str = None) -> Any: | |
| def upload_keyframe_preannotations(self, video_json_file_path: str) -> Any: | |
| \"\"\" | |
| Uploads pre-annotations for video project in video JSON format. | |
| :param video_json_file_path: Path to the video JSON file containing pre-annotations | |
| :return: Response from the API | |
| :raises LabellerrError: If file doesn't exist or upload fails | |
| \"\"\" |
|
|
||
| try: | ||
| # Validate if the file exists | ||
| if not os.path.exists(file_path): |
There was a problem hiding this comment.
The function should validate:
- File extension (must be .json)
- File is not empty
- JSON is valid before uploading
# Add before opening file
if not file_path.endswith('.json'):
raise LabellerrError("File must be a JSON file")
if os.path.getsize(file_path) == 0:
raise LabellerrError("File is empty")
| from labellerr.services.video_sampling import coco_to_video_json | ||
|
|
||
| # Input COCO JSON file path | ||
| coco_json_path = r"D:\Professional\Labellerr_SDK\SDKPython\labellerr\notebooks\export-#huy0VWY14med4McdKd6h.json" |
There was a problem hiding this comment.
🔴 Critical: Hardcoded Windows Path
This test file has hardcoded Windows paths that won't work in CI/CD or on other systems. Test files should:
- Use relative paths
- Accept command-line arguments
- Use Path objects for cross-platform compatibility
- Be in a proper tests/ directory with pytest
from pathlib import Path
import sys
if len(sys.argv) > 1:
coco_json_path = Path(sys.argv[1])
else:
coco_json_path = Path("test_data/export.json")
| from labellerr.core.projects.video_project import LabellerrProject | ||
|
|
||
| # Load environment variables from .env file | ||
| load_dotenv(r"D:\Professional\Labellerr_SDK\dev.env") |
There was a problem hiding this comment.
🔴 Critical: Hardcoded Paths and Missing Best Practices
Multiple issues:
- Hardcoded Windows path to .env file
- No error handling for missing environment variables
- Hardcoded project ID
- Should use pytest instead of main
This should be refactored into proper unit/integration tests in a tests/ directory.
| # Fetch all video files | ||
| video_files = self.fetch_files() | ||
| # Fetch all video files (convert generator to list) | ||
| video_files = list(self.fetch_files()) |
There was a problem hiding this comment.
Converting the generator to a list loads all video files into memory. For large datasets, this could cause memory issues.
Consider:
- Keep as generator and process in batches
- Add pagination support
- Stream process the files
# Process in batches
for video_file in self.fetch_files():
# Process one at a time
video_file.download_create_video_auto_cleanup()
| @@ -1,4 +1,4 @@ | |||
| BASE_URL = "https://api.labellerr.com" | |||
| BASE_URL = "https://api-gateway-qcb3iv2gaa-uc.a.run.app" | |||
There was a problem hiding this comment.
CRITICAL SECURITY ISSUE: The BASE_URL is hardcoded to a QA/staging environment (api-gateway-qcb3iv2gaa-uc.a.run.app). This should NOT be committed to the main branch as it will affect all production users.
Recommendation:
- Revert this to the production URL (
https://api.labellerr.com) - Use environment variables or configuration files for environment-specific URLs
- Add a clear warning comment if this is intentionally for testing
| try: | ||
| # Use file_id as folder name | ||
| folder_name = self.file_id | ||
| # Use [Dataset_id]+[File_id]+[File_name] as folder name |
There was a problem hiding this comment.
Code Quality Issue: The folder naming logic is overly complex and repeated in multiple places (lines 124-132, 292-298, 315-322, 359-363).
Recommendation: Extract this into a private method:
def _get_folder_name(self) -> str:
if self.dataset_id and self.file_name:
base_name = os.path.splitext(self.file_name)[0]
return f"{self.dataset_id}+{self.file_id}+{base_name}"
elif self.dataset_id:
return f"{self.dataset_id}+{self.file_id}"
else:
return self.file_idThis reduces code duplication and makes maintenance easier.
| @property | ||
| def fps(self): | ||
| """Get frames per second of the video.""" | ||
| return self.metadata.get("fps", 25) |
There was a problem hiding this comment.
Potential Bug: Using a hardcoded default FPS of 25 could cause issues. If the actual video has different FPS and this default is used in calculations, it will result in incorrect frame timing.
Recommendation: Consider raising an error if FPS is not available in metadata rather than silently falling back to a default, or at minimum log a warning.
|
|
||
| def download_create_video_auto_cleanup( | ||
| self, output_folder: str = "./Labellerr_datastets" | ||
| self, output_folder: str = "./Labellerr_datasets" |
There was a problem hiding this comment.
Typo in default path: "Labellerr_datastets" should be "Labellerr_datasets" (missing 'a').
| except Exception as e: | ||
| raise LabellerrError(f"Failed to delete key frames: {str(e)}") | ||
|
|
||
| def upload_keyframe_preannotations(self, video_json_file_path: str = None) -> Any: |
There was a problem hiding this comment.
Code Quality Issue: The docstring mentions deprecated parameters (annotation_format, annotation_file, conf_bucket, _async) that don't exist in the method signature. This is confusing and misleading.
Recommendation: Clean up the docstring to only document the actual parameter:
"""
Uploads pre-annotations for video project.
:param video_json_file_path: Path to the video JSON file containing pre-annotations
:return: Response from the API
"""
| # Process each video file | ||
| results = [] | ||
| for idx, filename in enumerate(video_files, 1): | ||
| file_path = os.path.join(dataset_path, filename) |
There was a problem hiding this comment.
Performance Issue: Using os.listdir() and filtering with list comprehension loads all files into memory. For directories with many files, this could be inefficient.
Recommendation: Use os.scandir() or Path.glob() for better performance:
video_files = [
f.name for f in os.scandir(dataset_path)
if f.is_file() and os.path.splitext(f.name)[1].lower() in VIDEO_EXTENSIONS
]
| # ============================================================================ | ||
|
|
||
|
|
||
| def _extract_video_name_and_frame(filename: str) -> tuple[str, int, int]: |
There was a problem hiding this comment.
Code Quality: The return type annotation uses tuple[str, int, int] (Python 3.10+ syntax) but the codebase might need to support older Python versions.
Recommendation: Verify Python version requirements. If supporting <3.10, use Tuple[str, int, int] from typing module.
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| video_path = r"D:\Professional\Labellerr_SDK\SDKPython\labellerr\notebooks\Labellerr_datasets\354681d3-034a-4d66-b070-365f4bd11d8a\2a8d96ca-9161-4dee-ad3b-a5faf301bc6c.mp4" |
There was a problem hiding this comment.
Security Issue: Hardcoded absolute Windows path in __main__ block. This exposes internal directory structure and won't work on other machines.
Recommendation: Remove or use a relative example path like "./test_video.mp4".
|
|
||
| if __name__ == "__main__": | ||
| # Example usage | ||
| video_path = r"D:\Professional\Labellerr_SDK\SDKPython\labellerr\notebooks\Labellerr_datasets\354681d3-034a-4d66-b070-365f4bd11d8a\2a8d96ca-9161-4dee-ad3b-a5faf301bc6c.mp4" |
There was a problem hiding this comment.
Security Issue: Same as ffmpeg_detect.py - hardcoded absolute Windows path exposing internal directory structure.
| from labellerr.services.video_sampling import coco_to_video_json | ||
|
|
||
| # Input COCO JSON file path | ||
| coco_json_path = r"D:\Professional\Labellerr_SDK\SDKPython\labellerr\notebooks\export-#huy0VWY14med4McdKd6h.json" |
There was a problem hiding this comment.
Security Issue: Hardcoded absolute Windows paths exposing internal directory structure. These test files should either:
- Use relative paths
- Load paths from environment variables
- Not be committed to the repository (should be in .gitignore)
| from labellerr.core.projects.video_project import LabellerrProject | ||
|
|
||
| # Load environment variables from .env file | ||
| load_dotenv(r"D:\Professional\Labellerr_SDK\dev.env") |
There was a problem hiding this comment.
Security Issue: Loading environment variables from a hardcoded absolute path. This path won't exist on other machines and exposes internal directory structure.
Recommendation: Either use default .env file in current directory or make it configurable.
| "from dotenv import dotenv_values\n", | ||
| "\n", | ||
| "config = dotenv_values(\".env\")\n", | ||
| "config = dotenv_values(\"../dev.env\")\n", |
There was a problem hiding this comment.
Code Quality Issue: The notebook contains hardcoded QA credentials in the config keys (QA_API_KEY, QA_API_SECRET, QA_CLIENT_ID). While these are loaded from env file, it suggests this notebook is configured for QA environment which shouldn't be in the main branch.
Recommendation: Use generic key names or document that this is a development/testing notebook.
Pull Request Review SummaryThis PR introduces video keyframe scene detection functionality with three detection algorithms (FFMPEG, PySceneDetect, SSIM) and COCO to Video JSON conversion utilities. While the core functionality is well-implemented, there are critical issues that must be addressed before merging. 🚨 Critical Issues (Must Fix)1. SECURITY: Hardcoded QA/Staging URL
|
1 participant
No description provided.