Skip to content

Improve video file check by excluding attached pictures #321

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.

Sign up for GitHub

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

Open
filipsworks wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve video file check by excluding attached pictures #321

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions src/modules/ffmpeg_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,23 +70,36 @@ def remove_audio_from_video(input_video_path: str, output_video_path: str) -> No
if result.returncode != 0:
raise Exception(f"FFmpeg audio removal failed: {result.stderr}")


def is_video_file(file_path: str) -> bool:
"""Check if file contains video streams using ffprobe"""
"""Check if file contains video streams using ffprobe (excluding attached pictures)"""
try:
_, ffprobe_path = get_ffmpeg_and_ffprobe_paths()

cmd = [
ffprobe_path,
"-v", "error",
"-select_streams", "v:0",
"-show_entries", "stream=codec_type",
"-show_entries", "stream=codec_type,disposition",
"-of", "default=noprint_wrappers=1:nokey=1",
file_path
]

result = subprocess.run(cmd, capture_output=True, text=True)
return result.returncode == 0 and result.stdout.strip() == "video"

if result.returncode != 0 or not result.stdout.strip():
return False

# Parse output to check for actual video (not attached picture)
lines = result.stdout.strip().split('\n')
if len(lines) < 2:
return False

codec_type = lines[0].strip()
disposition = lines[1].strip() if len(lines) > 1 else ""

# Return True only if it's video AND not an attached picture
return codec_type == "video" and "attached_pic" not in disposition
Comment on lines +88 to +101

@coderabbitai coderabbitai Bot May 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify ffprobe output format with nokey=1 vs without nokey

# Create a test file (if ffprobe and ffmpeg are available in the sandbox)
# For demonstration, show the command that would reveal the difference:

echo "=== Current command (with nokey=1) ==="
echo "ffprobe -v error -select_streams v:0 -show_entries stream=codec_type,disposition -of default=noprint_wrappers=1:nokey=1 <file>"
echo ""
echo "Expected output format (numeric values only, no keys):"
echo "video"
echo "0"
echo "0"
echo "..."
echo ""
echo "=== Recommended command (without nokey=1) ==="
echo "ffprobe -v error -select_streams v:0 -show_entries stream=codec_type,disposition:stream_disposition=attached_pic -of default=noprint_wrappers=1 <file>"
echo ""
echo "Expected output with keys:"
echo "codec_type=video"
echo "default=0"
echo "..."
echo "attached_pic=1"

Repository: rakuri255/UltraSinger

Length of output: 573

🏁 Script executed:

# First, examine the actual source file to understand full context
find . -type f -name "ffmpeg_helper.py" -o -name "*ffmpeg*" | head -20

Repository: rakuri255/UltraSinger

Length of output: 96

🏁 Script executed:

# Check if the file exists and read the relevant section
cat -n src/modules/ffmpeg_helper.py | sed -n '75,110p'

Repository: rakuri255/UltraSinger

Length of output: 1489

🏁 Script executed:

# Verify ffprobe availability and test actual output formats
which ffprobe ffmpeg

Repository: rakuri255/UltraSinger

Length of output: 111

🏁 Script executed:

# Test ffprobe output with nokey=1 vs without (if tools available)
if command -v ffprobe &> /dev/null; then
  echo "=== Testing ffprobe output formats ==="
  
  # Create a simple test video file if ffmpeg is available
  if command -v ffmpeg &> /dev/null; then
    # Generate a minimal test video
    ffmpeg -f lavfi -i testsrc=s=320x240:d=1 -f lavfi -i sine=f=1000:d=1 -c:v libx264 -c:a aac -shortest /tmp/test_video.mp4 2>/dev/null
    
    if [ -f /tmp/test_video.mp4 ]; then
      echo "Test video created successfully"
      echo ""
      echo "=== Output WITH nokey=1 ==="
      ffprobe -v error -select_streams v:0 -show_entries stream=codec_type,disposition -of default=noprint_wrappers=1:nokey=1 /tmp/test_video.mp4 2>/dev/null || echo "Command failed"
      echo ""
      echo "=== Output WITHOUT nokey=1 ==="
      ffprobe -v error -select_streams v:0 -show_entries stream=codec_type,disposition -of default=noprint_wrappers=1 /tmp/test_video.mp4 2>/dev/null || echo "Command failed"
    fi
  else
    echo "ffmpeg not available, cannot create test video"
  fi
else
  echo "ffprobe not available in sandbox"
fi

Repository: rakuri255/UltraSinger

Length of output: 98

🌐 Web query:

ffprobe nokey=1 output format default option documentation

💡 Result:

In ffprobe, the nokey option is a writer-specific parameter used to suppress the printing of field keys in the output [1][2]. To use it with the default output format, you must pass it as an option to the print_format (or -of) flag using the syntax: ffprobe -print_format default=nokey=1 [other_options] input_file [3] Key details regarding this option: 1. Syntax: The option must be attached to the writer name using an equals sign, and multiple options can be chained using colons [3]. For example, to hide both keys and section wrappers, use: ffprobe -print_format default=nokey=1:noprint_wrappers=1 input_file [3] 2. Default Behavior: By default, the nokey option is set to 0 [1][2]. When set to 1, ffprobe will output only the values of the fields rather than the key=value pairs [1][4]. 3. Writer Specifics: While nokey is available for the default writer, it is also commonly used with other writers like csv, where it may have different default behaviors (e.g., it defaults to 1 for the csv writer) [1][2]. Do not confuse this with the -skip_frame nokey option, which is a separate, unrelated flag used to instruct the decoder to skip non-key frames [3][5].

Citations:

🌐 Web query:

ffprobe disposition output format codec_type with noprint_wrappers nokey option

💡 Result:

To output only the codec_type of a stream using ffprobe while suppressing keys and section wrappers, use the following command structure [1][2][3]: ffprobe -v error -show_entries stream=codec_type -of default=noprint_wrappers=1:nokey=1 input_file Explanation of options: - -v error: Suppresses non-essential log output [2][3]. - -show_entries stream=codec_type: Specifies that only the codec_type field from the stream section should be displayed [4][5]. - -of default=...: Sets the output format to the default writer [4][6]. - noprint_wrappers=1 (or nw=1): Disables the printing of section headers and footers [4][7]. - nokey=1 (or nk=1): Disables the printing of the field key (e.g., "codec_type="), leaving only the value [4][1][3]. You can combine these options using a colon separator within the -of (or -print_format) argument [1][3].

Citations:

Critical: The disposition check will not work as intended.

The ffprobe format option nokey=1 suppresses field names, causing ffprobe to output only values without keys. When requesting stream=codec_type,disposition, the output will be:

  • Line 0: video (the codec_type value)
  • Lines 1+: Numeric values 0 or 1 for each disposition flag (default, dub, original_language, comment, lyrics, karaoke, forced, hearing_impaired, visual_impaired, clean_effects, attached_pic, timed_thumbnails)

The code at line 101 attempts to check "attached_pic" not in disposition against lines[1], which will be a numeric string like "0" or "1". The substring "attached_pic" will never be found, making it impossible to detect attached pictures.

Fix: Remove nokey=1 from line 83 and update the parsing logic to handle key=value pairs, or alternatively, query only the specific attached_pic disposition flag:

-            "-of", "default=noprint_wrappers=1:nokey=1",
+            "-of", "default=noprint_wrappers=1",

Then parse the key=value output to identify the codec_type and attached_pic disposition status.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/modules/ffmpeg_helper.py` around lines 88 - 101, The disposition parsing
fails because ffprobe was run with nokey=1 so lines contain only values (e.g.
"0"/"1") and the code checks for the literal "attached_pic". Change the ffprobe
invocation to include keys (remove nokey=1) or request key=value output, then
parse key=value pairs: extract codec_type from the line like "codec_type=video"
and find the disposition key "attached_pic=1" (or "attached_pic=0") and return
True only when codec_type == "video" and attached_pic != "1"; update the
variables used in this block (result, codec_type, disposition/attached_pic)
accordingly so the check is correct.


except Exception:
return False

Expand Down