Skip to content

ffmpeg: tolerate trailing CSV separator from side-data streams (iPhone .mov) - #31

Open
siraustin wants to merge 1 commit into
antirez:mainfrom
siraustin:ffprobe-side-data
Open

ffmpeg: tolerate trailing CSV separator from side-data streams (iPhone .mov)#31
siraustin wants to merge 1 commit into
antirez:mainfrom
siraustin:ffprobe-side-data

Conversation

@siraustin

Copy link
Copy Markdown

h3_ffprobe_visual_size() runs ffprobe ... -of csv=p=0:s=x and rejects any non-whitespace after the parsed WxH. For streams carrying side data — e.g. the display-matrix side data present in every iPhone-shot .mov — ffprobe's CSV writer appends a trailing field separator, so the output is 3840x2160x and the probe fails with "FFprobe returned an invalid visual size" on files that are perfectly valid inputs.

Repro: ffprobe -v error -select_streams v:0 -show_entries stream=width,height -of csv=p=0:s=x IMG_xxxx.mov on any iPhone clip → 3840x2160x.

Fix: tolerate stray separator characters (and whitespace) after the parsed size; anything else is still rejected, and the width/height sanity checks below are unchanged. Verified against the failing .mov (now parses 3840x2160) and a plain jpeg (unchanged, 1408x1408).

🤖 Generated with Claude Code

@siraustin

Copy link
Copy Markdown
Author

Updated after an external review pass: the tolerance is now exactly one optional trailing separator before whitespace (the shape FFmpeg's compact writer actually produces for an unselected nested side_data_list, present in FFmpeg ≥ 5.1; absent in 4.4), rather than accepting arbitrary stray x characters. Verified against a display-matrix .mov (3840x2160x → parses) and a plain jpeg (unchanged).

One adjacent observation from the same review, deliberately left out of this PR to keep it minimal: stream=width,height reports coded dimensions, but the decode paths don't pass -noautorotate, so a file with a 90°/270° display-matrix rotation probes landscape while FFmpeg decodes it portrait — the canvas math then stretches the frame. The files that trip the parser bug here (iPhone .movs) are exactly the family that can carry such rotations. Happy to open a separate issue/PR for rotation-aware probing (e.g. stream_side_data=rotation + swap on ~90/270) if wanted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant