Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

[PATCH] Completion/Unix/_ffmpeg: fix crash and update for modern ffmpeg



The _ffmpeg completion function has three bugs that surface with modern
ffmpeg (n6+/n7+/n8+):

Bug 1 — zsh SEGV on tab completion
-----------------------------------
The function builds its _arguments spec dynamically by piping `ffmpeg -h`
output through a while loop that runs in the current shell process (unlike
bash, zsh runs the right-hand side of a pipeline in the current process by
default).  The loop uses (#b) back-reference patterns
in [[ ]] tests:

  if [[ $lastopt_description == (#b)'<'(?##)'>'* ]]; then

Modern ffmpeg's -h output includes options with long descriptions (e.g.
-print_graphs_format) that trigger a crash in pattryrefs during interactive
tab completion, killing the interactive shell.  The pattern applied to the
long description string is:

  lastopt_description="<format>  set the output printing format \
(available formats are: default, compact, csv, flat, ini, json, xml, \
mermaid, mermaidhtml)"
  [[ $lastopt_description == (#b)'<'(?##)'>'* ]]

Bug 2 — codec lists always empty
---------------------------------
_ffmpeg_vcodecs, _ffmpeg_acodecs, and _ffmpeg_scodecs filter `ffmpeg -codecs`
output with a pattern that matches the old 6-flag column format (D,E,V,S,D,T):

  :#[[:space:]][D[:space:]][E[:space:]]V[S[:space:]][D[:space:]][T[:space:]]...

Since ffmpeg 5.x absent flags are shown as '.' instead of a space.  The
character classes in the pattern — [D[:space:]], [E[:space:]], etc. — accept
only the literal letter or a space; a '.' satisfies neither, so every codec
line fails to match and the lists are always empty.  Tab-completing -vcodec
produces only "copy".

Bug 3 — malformed _arguments specs from option names containing brackets
-------------------------------------------------------------------------
Modern ffmpeg has options where the optional stream specifier is part of
the printed name: -c[:<stream_spec>], -metadata[:<spec>], -r[:<stream_spec>].
The while loop extracts the raw token as lastopt, producing strings like
"-c[:<stream_spec>]".  When concatenated into _arguments specs, the '[' and
']' are interpreted as the description delimiter, generating malformed specs.

Fix
---
Replace the fragile `ffmpeg -h` parsing loop with a static _arguments call
covering the stable option set.  Rewrite the codec and format helpers to use
the dedicated query flags (-encoders, -formats), whose structured output is
designed for programmatic use and is far more stable than the free-form -h
text.  Use awk for extraction to keep the patterns readable and avoid the
(#b) constructs that trigger the zsh pattryrefs crash.

Also fixed in passing:
- _ffmpeg_scodecs had three copy-paste errors from _ffmpeg_vcodecs: the
  _call_program tag ("video-codecs" → "subtitle-codecs"), the _wanted tag
  ("ffmpeg-video-codecs" → "ffmpeg-subtitle-codecs"), and the description
  string ("force video codec" → "force subtitle codec").
- _ffmpeg_pix_fmts had an unused `local pix_fmts` variable — removed.
- Added a static -bsf spec so _ffmpeg_bsfs (unchanged from original) is no
  longer dead code; -bsf does not appear in the basic `ffmpeg -h` output
  that the dynamic parser reads (only in `ffmpeg -h full`), so _ffmpeg_bsfs
  was never reached.
- Restored the * (repeatable) prefix on -f, -pix_fmt, -vpre, -apre, -spre,
  which the original dynamic parser added explicitly.  This matters for
  invocations like: ffmpeg -f lavfi -i testsrc -f mp4 output.mp4

Tested with: zsh 5.9, ffmpeg n8.1.1 (Arch Linux).
Codec helpers verified to return correct counts (136 video, 81 audio,
11 subtitle encoders; 430 formats; 267 pixel formats; 50 bitstream filters).

---
 Completion/Unix/_ffmpeg | 268 ++++++++++++++++++++++---------------------------------------
 1 file changed, 104 insertions(+), 164 deletions(-)

--- a/Completion/Unix/_ffmpeg
+++ b/Completion/Unix/_ffmpeg
@@ -10,31 +10,39 @@
 }

 (( $+functions[_ffmpeg_acodecs] )) || _ffmpeg_acodecs() {
-    local acodecs
-    acodecs=(copy ${${(M)${(f)"$(_call_program audio-codecs $words[1] -codecs 2>/dev/null)"}:#[[:space:]][D[:space:]][E[:space:]]A[S[:space:]][D[:space:]][T[:space:]][[:space:]][^[:space:]]##*}//(#b)????????([^[:space:]]##)*/$match[1]})
-    _wanted ffmpeg-audio-codecs expl 'force audio codec (''copy'' to copy stream)' compadd -a acodecs
+    local -a acodecs
+    acodecs=(copy ${(f)"$(_call_program audio-codecs \
+        $words[1] -encoders 2>/dev/null | awk 'NR>10 && /^ A/{print $2}')"})
+    _wanted ffmpeg-audio-codecs expl \
+        'force audio codec (''copy'' to copy stream)' compadd -a acodecs
 }

 (( $+functions[_ffmpeg_vcodecs] )) || _ffmpeg_vcodecs() {
-    local vcodecs
-    vcodecs=(copy ${${(M)${(f)"$(_call_program video-codecs $words[1] -codecs 2>/dev/null)"}:#[[:space:]][D[:space:]][E[:space:]]V[S[:space:]][D[:space:]][T[:space:]][[:space:]][^[:space:]]##*}//(#b)????????([^[:space:]]##)*/$match[1]})
-    _wanted ffmpeg-video-codecs expl 'force video codec (''copy'' to copy stream)' compadd -a vcodecs
+    local -a vcodecs
+    vcodecs=(copy ${(f)"$(_call_program video-codecs \
+        $words[1] -encoders 2>/dev/null | awk 'NR>10 && /^ V/{print $2}')"})
+    _wanted ffmpeg-video-codecs expl \
+        'force video codec (''copy'' to copy stream)' compadd -a vcodecs
 }

 (( $+functions[_ffmpeg_scodecs] )) || _ffmpeg_scodecs() {
-    local scodecs
-    scodecs=(copy ${${(M)${(f)"$(_call_program video-codecs $words[1] -codecs 2>/dev/null)"}:#[[:space:]][D[:space:]][E[:space:]]S[S[:space:]][D[:space:]][T[:space:]][[:space:]][^[:space:]]##*}//(#b)????????([^[:space:]]##)*/$match[1]})
-    _wanted ffmpeg-video-codecs expl 'force video codec (''copy'' to copy stream)' compadd -a scodecs
+    local -a scodecs
+    scodecs=(copy ${(f)"$(_call_program subtitle-codecs \
+        $words[1] -encoders 2>/dev/null | awk 'NR>10 && /^ S/{print $2}')"})
+    _wanted ffmpeg-subtitle-codecs expl \
+        'force subtitle codec (''copy'' to copy stream)' compadd -a scodecs
 }

 (( $+functions[_ffmpeg_formats] )) || _ffmpeg_formats() {
-    local formats
-    formats=(${(ou)${=${(s:,:)${${(M)${(f)"$(_call_program formats $words[1] -formats 2>/dev/null)"}:#[[:space:]][D[:space:]][E[:space:]][[:space:]][^[:space:]]##*}//(#b)????([^[:space:]]##)*/$match[1]}}}})
+    local _out
+    _out=$(_call_program formats $words[1] -formats 2>/dev/null | \
+        awk 'NR>4 {name=($2=="d")?$3:$2; n=split(name,a,","); for(i=1;i<=n;i++) if(a[i]~/^[a-z0-9]/) print a[i]}')
+    local -a formats
+    formats=(${(ou)${(f)_out}})
     _wanted ffmpeg-formats expl 'force format' compadd -a formats
 }

 (( $+functions[_ffmpeg_pix_fmts] )) || _ffmpeg_pix_fmts() {
-    local pix_fmts
     _wanted ffmpeg-pix-fmts expl 'pixel format' compadd "$@" - \
         ${${${(M)${(f)"$(_call_program formats $words[1] -pix_fmts 2>/dev/null)"}:#[I.][O.][H.][P.][B.] [^=[:space:]]*}#* }%% *}
 }
@@ -45,156 +53,88 @@
     _wanted ffmpeg-bsfs expl 'set bitstream filter' compadd -a bsfs
 }

-typeset -A _ffmpeg_flags
-
-(( $+functions[_ffmpeg_flag_options] )) || _ffmpeg_flag_options() {
-    local expl
-    _wanted options expl 'flag' compadd -S '' -- {-,+}${^flag_options}
-}
-
-(( $+functions[_ffmpeg_more_flag_options] )) || _ffmpeg_more_flag_options() {
-    compset -p $1 && _ffmpeg_flag_options
-}
-
-(( $+functions[_ffmpeg_new_flag_options] )) || _ffmpeg_new_flag_options() {
-    compset -P '*' && _ffmpeg_flag_options
-}
-
-(( $+functions[_ffmpeg_flags] )) || _ffmpeg_flags() {
-    local -a flag_options
-    eval "flag_options=(\${=_ffmpeg_flags[$1]})"
-
-    local match mbegin mend
-    integer ret=1
-
-    if [[ $PREFIX = (#b)(*)[-+]([^-+]#) ]]; then
-        if [[ -n ${flag_options[(R)$match[2]]} ]]; then
-            _ffmpeg_new_flag_options && ret=0
-        fi
-        if [[ -n ${flag_options[(R)$match[2]?*]} ]]; then
-            _ffmpeg_more_flag_options ${#match[1]} && ret=0
-        fi
-    else
-        _ffmpeg_flag_options && ret=0
-    fi
-
-    return ret
-}
-
-(( $+functions[_ffmpeg_register_lastopt_values] )) || _ffmpeg_register_lastopt_values() {
-    if (( lastopt_takesargs )); then
-        lastopt+=":$lastopt_description:"
-        if (( $#lastopt_values )); then
-            if [[ $lastopt_type == flags ]]; then
-                lastopt="*$lastopt"
-                flagtype=${${lastopt%%:*}#-}
-                lastopt+="->$flagtype"
-                _ffmpeg_flags[$flagtype]="${lastopt_values[*]}"
-            else
-                lastopt+="(${lastopt_values[*]})"
-            fi
-        fi
-    fi
-    _ffmpeg_argspecs+=$lastopt
-}
-
-local -a _ffmpeg_argspecs
-{
-    local lastopt REPLY
-    local lastopt_description
-    local lastopt_takesargs
-    local lastopt_type
-    local -a lastopt_values
-
-    _call_program options $words[1] -h 2>/dev/null | while IFS=$'\n' read -r; do
-        if [[ $REPLY == -* ]]; then
-            [[ -n $lastopt ]] && _ffmpeg_register_lastopt_values
-            lastopt=${REPLY%%[[:space:]]*}
-            lastopt_description=${REPLY##-[^[:space:]]##[[:space:]]##}
-            if [[ $lastopt_description == (#b)'<'(?##)'>'* ]]; then
-                lastopt_type=$match[1]
-                lastopt_description=${lastopt_description##<[^[:space:]]##>[[:space:]]##[^[:space:]]##[[:space:]]#}
-                if [[ -z $lastopt_description ]]; then
-                    lastopt_description=$lastopt
-                fi
-                lastopt_description=${lastopt_description//:/\\:}
-            elif [[ $lastopt_description == [^[:space:]]##[[:space:]][[:space:]]* ]]; then
-                local example=${lastopt_description%% *}
-                example=${example//:/\\:}
-                lastopt_description=${lastopt_description##[^[:space:]]##[[:space:]]##}
-                lastopt_description=${lastopt_description//:/\\:}
-                if [[ $example == filename ]]; then
-                    lastopt_takesargs=0
-                    lastopt+=":$lastopt_description:_files"
-                elif [[ $lastopt == -[asv]pre ]]; then
-                    lastopt_takesargs=0
-                    lastopt="*$lastopt"
-                    lastopt+=": :_ffmpeg_presets"
-                elif [[ $lastopt == -acodec ]]; then
-                    lastopt_takesargs=0
-                    lastopt+=": :_ffmpeg_acodecs"
-                elif [[ $lastopt == -vcodec ]]; then
-                    lastopt_takesargs=0
-                    lastopt+=": :_ffmpeg_vcodecs"
-                elif [[ $lastopt == -scodec ]]; then
-                    lastopt_takesargs=0
-                    lastopt+=": :_ffmpeg_scodecs"
-                elif [[ $lastopt == -f ]]; then
-                    lastopt_takesargs=0
-                    lastopt="*$lastopt"
-                    lastopt+=": :_ffmpeg_formats"
-                elif [[ $lastopt == -pix_fmt ]]; then
-                    lastopt_takesargs=0
-                    lastopt="*$lastopt"
-                    lastopt+=":set pixel format:_ffmpeg_pix_fmts"
-                elif [[ $example == bitstream_filter ]]; then
-                    lastopt_takesargs=0
-                    lastopt+=": :_ffmpeg_bsfs"
-                else
-                    lastopt_takesargs=1
-                    lastopt_description+=" ($example)"
-                fi
-            else
-                lastopt_takesargs=0
-                if [[ $lastopt == -vfilters ]]; then
-                    lastopt+=": :->vfilters"
-                fi
-            fi
-            lastopt_values=()
-        elif [[ $REPLY == ' '* ]]; then
-            REPLY=${REPLY##[[:space:]]##}
-            REPLY=${REPLY%%[[:space:]]##*}
-            lastopt_takesargs=1
-            lastopt_values+=$REPLY
-        fi
-    done
-    [[ -n $lastopt ]] && _ffmpeg_register_lastopt_values
-}
-
 _arguments -C -S \
-    "${_ffmpeg_argspecs[@]}" \
-    '*:output file:_files' \
-    && return
-
-[[ "$state" == "vfilters" ]] &&
-    _values -s , -S = 'video filter' \
-    'aspect:set aspect ratio (rational number X\:Y or decimal number):' \
-    'crop:crop input video (x\:y\:width\:height):' \
-    'format: :_sequence -s : _ffmpeg_pix_fmts' \
-    'noformat: :_sequence -s : _ffmpeg_pix_fmts' \
-    'null' \
-    'pad:add pads to the input image (width\:height\:x\:y\:color_string):' \
-    'pixelaspect:set pixel aspect ratio (rational number X\:Y or decimal number):' \
-    'scale:scale input video (width\:height):' \
-    'slicify:output slice height ("random" or a number of pixels):' \
-    'unsharp:luma_x\:luma_y\:luma_amount\:chroma_x\:chroma_y\:chroma_amount:' \
-    'vflip' \
-    'buffer' \
-    'nullsrc' \
-    'nullsink' \
-    && return
-
-[[ -n $state && -n $_ffmpeg_flags[$state] ]] &&
-    _ffmpeg_flags $state && return
-
-return 1
+    '(-y -n)-y[overwrite output files without asking]' \
+    '(-y -n)-n[never overwrite output files]' \
+    '-v[set logging level]:level:(quiet panic fatal error warning info verbose debug trace)' \
+    '-loglevel[set logging level]:level:(quiet panic fatal error warning info verbose debug trace)' \
+    '-report[generate a report]' \
+    '-hide_banner[suppress printing banner]' \
+    '-cpuflags[force specific cpu flags]:flags' \
+    '-stats[print encoding progress and statistics]' \
+    '-nostats[suppress encoding progress and statistics]' \
+    '-progress[write program-readable progress information]:URL' \
+    '-benchmark[add benchmarking information to the report]' \
+    '-timelimit[exit after specified number of seconds of CPU time]:seconds' \
+    '-dump[dump each input packet to stderr]' \
+    '-hex[dump each input packet in hexadecimal]' \
+    '-re[read input at native frame rate]' \
+    '-stream_loop[set number of times to loop the input]:count' \
+    '-i[input file or URL]:input file:_files' \
+    '*-f[force container format]:format:_ffmpeg_formats' \
+    '-c[select encoder or decoder]:codec' \
+    '-codec[select encoder or decoder]:codec' \
+    '-t[stop transcoding after duration]:duration' \
+    '-to[stop transcoding at time]:time' \
+    '-fs[set output file size limit in bytes]:bytes' \
+    '-ss[start at time offset]:time' \
+    '-sseof[start at time offset relative to EOF]:time' \
+    '-seek_timestamp[seek by timestamp with -ss when set to 1]:flag' \
+    '-itsoffset[set input time offset]:time' \
+    '-timestamp[set the recording timestamp]:time' \
+    '-metadata[add metadata to the output file]:key=value' \
+    '-program[add a program with specified streams]:title=outfile,key=value,...' \
+    '-target[specify target file type]:type:(vcd svcd dvd dv dv50)' \
+    '-apad[pad audio output with silence]' \
+    '-frames[stop writing after specified number of frames]:count' \
+    '-filter[set filtergraph for a stream]:filtergraph' \
+    '-filter_script[read filtergraph from file]:file:_files' \
+    '-reinit_filter[reinit filtergraph on input parameter changes]:flag' \
+    '-discard[discard matching stream packets]:type' \
+    '-disposition[set disposition flags for a stream]:flags' \
+    '-map[set input stream mapping]:stream_specifier' \
+    '-map_metadata[set metadata information of outfile]:outfile_spec' \
+    '-map_chapters[read chapters from input file]:input_file_index' \
+    '-copyts[copy timestamps from input to output]' \
+    '-start_at_zero[shift timestamps to start at 0 with -copyts]' \
+    '-copytb[copy input stream time base when stream copying]:mode' \
+    '-shortest[finish encoding when the shortest input stream ends]' \
+    '-dts_delta_threshold[timestamp discontinuity delta threshold]:threshold' \
+    '-muxdelay[set the maximum demux-decode delay in seconds]:seconds' \
+    '-muxpreload[set the initial demux-decode delay in seconds]:seconds' \
+    '-streamid[force a stream identifier]:ost_index:newval' \
+    '-bitexact[enable bitexact mode]' \
+    '-xerror[exit on error]' \
+    '-strict[how strictly to follow standards]:level:(very strict normal unofficial experimental)' \
+    '-max_muxing_queue_size[maximum packets in the muxing queue]:count' \
+    '-threads[number of threads to use]:count' \
+    '-vn[disable video]' \
+    '-vcodec[force video codec]:codec:_ffmpeg_vcodecs' \
+    '-vf[set video filtergraph]:filtergraph' \
+    '-b[set video bitrate]:bitrate' \
+    '-r[set video frame rate]:rate' \
+    '-s[set video frame size]:WxH' \
+    '-aspect[set video aspect ratio]:ratio' \
+    '*-pix_fmt[set pixel format]:format:_ffmpeg_pix_fmts' \
+    '-vframes[set number of video frames to output]:count' \
+    '-g[set group of picture size]:size' \
+    '*-vpre[set video preset options]:preset:_ffmpeg_presets' \
+    '-an[disable audio]' \
+    '-acodec[force audio codec]:codec:_ffmpeg_acodecs' \
+    '-af[set audio filtergraph]:filtergraph' \
+    '-ab[set audio bitrate]:bitrate' \
+    '-ar[set audio sampling rate in Hz]:rate' \
+    '-ac[set number of audio channels]:channels' \
+    '-aq[set audio quality]:quality' \
+    '-aframes[set number of audio frames to output]:count' \
+    '-atag[force audio tag or fourcc]:fourcc' \
+    '*-apre[set audio preset options]:preset:_ffmpeg_presets' \
+    '-sn[disable subtitle]' \
+    '-scodec[force subtitle codec]:codec:_ffmpeg_scodecs' \
+    '-stag[force subtitle tag or fourcc]:fourcc' \
+    '-fix_sub_duration[fix subtitle duration]' \
+    '-canvas_size[set canvas size]:WxH' \
+    '*-spre[set subtitle preset options]:preset:_ffmpeg_presets' \
+    '-bsf[set bitstream filter]:bitstream filter:_ffmpeg_bsfs' \
+    '-dn[disable data streams]' \
+    '*:output file:_files'


Messages sorted by: Reverse Date, Date, Thread, Author