PR #23978 opened by cdcxd URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23978 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23978.patch
# Summary of changes Change the eb_receive_packet to drain available packet when possible, not only attempt after eb_send_frame Reason: The current implementation only attempts svt_av1_enc_get_packet after successful ff_encode_get_frame + eb_send_frame . Meaning we can only get 0 or 1 packet after sending a frame. The unharvested packet count will never decrease. It won't catch up burst output packets either. SVT-AV1 releases packets in mini-GOP sized bursts — measured at up to 39 at once. So finished packets are withheld from the application until it supplies more input. Can't drain it asap. I changed it to always try svt_av1_enc_get_packet if possible. In the LOW_DELAY and rtc mode (available since v3.1.0), the function will block, so we can't attempt it. # Verification - fate: fate-libsvtav1-hdr10 - ffmpeg cli, I built the patched one locally, asked coding agent to draft byte-identical comparison. It did pass, the script provenance is https://code.ffmpeg.org/cdcxd/FFmpeg/src/branch/libsvtav1-drain-first/verification_provenance/verify.sh <!-- If this PR requires new FATE test samples, attach them to the PR and list their target paths below (relative to the fate-suite root). Attached filenames must match the sample's filename: ```fate-samples # e.g. vorbis/new-sample.ogg ``` --> From 41dc7a52e74488b58c8cca8e1fedc38d88eb9bd1 Mon Sep 17 00:00:00 2001 From: cdong <[email protected]> Date: Sat, 1 Aug 2026 20:56:59 -0700 Subject: [PATCH 1/2] Change eb_receive_packet to drain available packet when possible, not only attempt after eb_send_frame --- libavcodec/libsvtav1.c | 44 ++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c index 0bcc176243..56133b6b11 100644 --- a/libavcodec/libsvtav1.c +++ b/libavcodec/libsvtav1.c @@ -653,22 +653,46 @@ static int eb_receive_packet(AVCodecContext *avctx, AVPacket *pkt) EbErrorType svt_ret; AVBufferRef *ref; int ret = 0; + int drain_packet_first, have_packet = 0; if (svt_enc->eos_flag == EOS_RECEIVED) return AVERROR_EOF; - ret = ff_encode_get_frame(avctx, frame); - if (ret < 0 && ret != AVERROR_EOF) - return ret; - if (ret == AVERROR_EOF) - frame = NULL; + /* Take any available packet, don't let them accumulate in output fifo. + * + * svt_av1_enc_get_packet() is documented in EbSvtAv1Enc.h to block when + * pic_send_done is set or when the library runs in low delay, so only poll + * ahead of sending when it is guaranteed not to. The library's + * copy_api_from_app() forces low delay for rtc without reflecting it in + * the configuration we hold, hence the separate check; + * */ + int drain_packet_first = svt_enc->eos_flag == EOS_NOT_REACHED && + svt_enc->enc_params.pred_structure != LOW_DELAY +#if SVT_AV1_CHECK_VERSION(3, 1, 0) + && !svt_enc->enc_params.rtc +#endif + ; - ret = eb_send_frame(avctx, frame); - if (ret < 0) - return ret; - av_frame_unref(svt_enc->frame); + if (drain_packet_first) { + svt_ret = svt_av1_enc_get_packet(svt_enc->svt_handle, &headerPtr, svt_enc->eos_flag); + have_packet = svt_ret != EB_NoErrorEmptyQueue; + } + + if (!have_packet) { + ret = ff_encode_get_frame(avctx, frame); + if (ret < 0 && ret != AVERROR_EOF) + return ret; + if (ret == AVERROR_EOF) + frame = NULL; + + ret = eb_send_frame(avctx, frame); + if (ret < 0) + return ret; + av_frame_unref(svt_enc->frame); + + svt_ret = svt_av1_enc_get_packet(svt_enc->svt_handle, &headerPtr, svt_enc->eos_flag); + } - svt_ret = svt_av1_enc_get_packet(svt_enc->svt_handle, &headerPtr, svt_enc->eos_flag); if (svt_ret == EB_NoErrorEmptyQueue) return AVERROR(EAGAIN); else if (svt_ret != EB_ErrorNone) -- 2.52.0 From 7c24cdd019a2b3fa7aabf5292b2aef61632ea029 Mon Sep 17 00:00:00 2001 From: cdong <[email protected]> Date: Sat, 1 Aug 2026 21:49:55 -0700 Subject: [PATCH 2/2] push verify.sh as provenance --- verification_provenance/verify.sh | 265 ++++++++++++++++++++++++++++++ 1 file changed, 265 insertions(+) create mode 100755 verification_provenance/verify.sh diff --git a/verification_provenance/verify.sh b/verification_provenance/verify.sh new file mode 100755 index 0000000000..f124e2eddc --- /dev/null +++ b/verification_provenance/verify.sh @@ -0,0 +1,265 @@ +#!/usr/bin/env bash +# A/B verification for the libsvtav1 drain-first patch. +# +# baseline : /opt/ffmpeg-base +# patched : /opt/ffmpeg-patched +# +# Acceptance criteria (each test prints PASS/FAIL and the script exits non-zero +# if any FAIL): +# 1. every encode completes within its timeout in both builds (no deadlock) +# 2. bitstreams are byte-identical between the two builds (no output change) +# 3. packet/frame counts and pts/dts ordering are preserved +# 4. the decoded output is identical (framemd5) +# 5. the patched build actually drains: drain_probe reports +# max_packets_per_send > 1, while the baseline reports exactly 1 +# +# Usage: verify.sh [duration_seconds] [timeout_seconds] + +set -uo pipefail + +DUR=${1:-10} +TMO=${2:-300} +OUT=${OUT:-/work/results} +BASE=/opt/ffmpeg-base/bin +PATCHED=/opt/ffmpeg-patched/bin + +mkdir -p "$OUT" + +# Two verify runs sharing $OUT will overwrite each other's .ivf files mid-write +# and produce results that look like encoder corruption. Refuse to run instead. +exec 9>"$OUT/.lock" +if ! flock -n 9; then + echo "ERROR: another verify run is using $OUT. Use a different -v mount or OUT=." >&2 + exit 2 +fi + +fails=0 +pass() { printf ' \033[32mPASS\033[0m %s\n' "$*"; } +fail() { printf ' \033[31mFAIL\033[0m %s\n' "$*"; fails=$((fails+1)); } +hdr() { printf '\n\033[1m== %s\033[0m\n' "$*"; } + +hdr "environment" +echo " SVT-AV1 $(cat /opt/svt-av1.rev 2>/dev/null) $(pkg-config --modversion SvtAv1Enc 2>/dev/null)" +echo " FFmpeg $(cat /opt/ffmpeg.rev 2>/dev/null)" +echo " patch $(cat /opt/applied-patch.stat 2>/dev/null | tr '\n' ' ')" +"$BASE/ffmpeg" -hide_banner -version | head -1 | sed 's/^/ base /' +"$PATCHED/ffmpeg" -hide_banner -version | head -1 | sed 's/^/ patched /' + +# --------------------------------------------------------------- sources ----- +hdr "generating test sources (${DUR}s)" +SRC_Y4M=$OUT/src.y4m +SRC_MP4=$OUT/src264.mp4 +"$BASE/ffmpeg" -hide_banner -loglevel error -y \ + -f lavfi -i "testsrc2=size=640x360:rate=30:duration=$DUR" \ + -pix_fmt yuv420p "$SRC_Y4M" +"$BASE/ffmpeg" -hide_banner -loglevel error -y \ + -f lavfi -i "testsrc2=size=1280x720:rate=30:duration=$DUR" \ + -c:v libx264 -preset veryfast -crf 28 -pix_fmt yuv420p "$SRC_MP4" +ls -l "$SRC_Y4M" "$SRC_MP4" | sed 's/^/ /' + +# --------------------------------------------------------------- matrix ------ +# name | input | encoder args +# NONDET=1 marks a configuration whose output SVT-AV1 does not reproduce +# bit-exactly from run to run even with an unchanged binary (verified for VBR: +# three consecutive baseline runs give three different md5s, while CRF is +# bit-exact). For those, byte identity is meaningless; the suite checks +# structural equivalence instead. +run_case() { + local name=$1 input=$2; shift 2 + local args=("$@") + local rc_b rc_p t_b t_p md5_b md5_p + local nondet=${NONDET:-0} + + printf '\n-- case %s\n args: %s\n' "$name" "${args[*]}" + + for v in base patched; do + local bin=$BASE; [ "$v" = patched ] && bin=$PATCHED + local start=$SECONDS + timeout -k 5 "$TMO" "$bin/ffmpeg" -hide_banner -loglevel error -y \ + -i "$input" "${args[@]}" "$OUT/$name.$v.ivf" \ + > "$OUT/$name.$v.log" 2>&1 + local rc=$? + local el=$((SECONDS-start)) + if [ "$v" = base ]; then rc_b=$rc; t_b=$el; else rc_p=$rc; t_p=$el; fi + if [ $rc -eq 124 ] || [ $rc -eq 137 ]; then + fail "$name/$v TIMED OUT after ${TMO}s (possible deadlock)" + elif [ $rc -ne 0 ]; then + fail "$name/$v exited $rc" + sed 's/^/ /' "$OUT/$name.$v.log" | head -20 + fi + done + [ "${rc_b:-1}" -eq 0 ] && [ "${rc_p:-1}" -eq 0 ] || return 1 + pass "$name both builds completed (base ${t_b}s, patched ${t_p}s)" + + # 2. bitstream identity + md5_b=$(md5sum < "$OUT/$name.base.ivf") + md5_p=$(md5sum < "$OUT/$name.patched.ivf") + if [ "$nondet" = 1 ]; then + # size within 2% is all that can be asserted here + local sb sp delta + sb=$(stat -c%s "$OUT/$name.base.ivf"); sp=$(stat -c%s "$OUT/$name.patched.ivf") + delta=$(( (sb > sp ? sb - sp : sp - sb) * 100 / sb )) + if [ "$delta" -le 2 ]; then + pass "$name size within ${delta}% (bit-exactness N/A: SVT non-deterministic here)" + else + fail "$name size differs by ${delta}% (base $sb vs patched $sp)" + fi + elif [ "$md5_b" = "$md5_p" ]; then + pass "$name bitstream identical (${md5_b%% *})" + else + fail "$name bitstream DIFFERS (base ${md5_b%% *} vs patched ${md5_p%% *})" + fi + + # 3. packet count + pts/dts monotonicity + for v in base patched; do + local n + n=$("$PATCHED/ffprobe" -hide_banner -loglevel error -select_streams v:0 \ + -show_entries packet=pts,dts -of csv=p=0 "$OUT/$name.$v.ivf" \ + > "$OUT/$name.$v.pkts.csv"; wc -l < "$OUT/$name.$v.pkts.csv") + echo " $v: $n packets" + done + if cmp -s "$OUT/$name.base.pkts.csv" "$OUT/$name.patched.pkts.csv"; then + pass "$name packet timestamps identical" + else + fail "$name packet timestamps differ" + fi + if sort -t, -k1,1n -c "$OUT/$name.patched.pkts.csv" 2>/dev/null; then + pass "$name pts non-decreasing" + else + fail "$name pts NOT monotonic" + fi + + # 4. decoded output identity + for v in base patched; do + "$PATCHED/ffmpeg" -hide_banner -loglevel error -y -c:v libdav1d \ + -i "$OUT/$name.$v.ivf" -f framemd5 "$OUT/$name.$v.framemd5" \ + >> "$OUT/$name.$v.log" 2>&1 + done + local nb_b nb_p + nb_b=$(grep -c '^[0-9]' "$OUT/$name.base.framemd5") + nb_p=$(grep -c '^[0-9]' "$OUT/$name.patched.framemd5") + if [ "$nondet" = 1 ]; then + [ "$nb_b" = "$nb_p" ] \ + && pass "$name same decoded frame count ($nb_p)" \ + || fail "$name decoded frame count differs ($nb_b vs $nb_p)" + elif cmp -s "$OUT/$name.base.framemd5" "$OUT/$name.patched.framemd5"; then + pass "$name decoded frames identical ($nb_p frames)" + else + fail "$name decoded frames differ" + fi +} + +hdr "encode matrix (A/B, timeout ${TMO}s per run)" + +run_case raw-p8 "$SRC_Y4M" -c:v libsvtav1 -preset 8 -crf 40 -g 120 +run_case raw-p12 "$SRC_Y4M" -c:v libsvtav1 -preset 12 -crf 40 -g 120 +run_case raw-p4 "$SRC_Y4M" -c:v libsvtav1 -preset 4 -crf 40 -g 120 +# deep hierarchy = biggest output bursts, this is where the pool pressure is +run_case hier5 "$SRC_Y4M" -c:v libsvtav1 -preset 8 -crf 40 -g 240 \ + -svtav1-params hierarchical-levels=5:lookahead=120 +run_case tenbit "$SRC_Y4M" -c:v libsvtav1 -preset 8 -crf 40 -pix_fmt yuv420p10le +# VBR: CBR/maxrate are rejected by SVT-AV1 outside low delay, so exercise the +# rate-control path with a plain target bitrate. Its output is not reproducible +# run-to-run in SVT-AV1 v4.2.0, hence NONDET. +NONDET=1 run_case vbr "$SRC_Y4M" -c:v libsvtav1 -preset 8 -b:v 2M +unset NONDET + +# Self-check that backs the NONDET classification above: same binary, twice. +hdr "determinism self-check (same binary, two runs)" +for mode in "-crf 40" "-b:v 2M"; do + for i in 1 2; do + "$BASE/ffmpeg" -hide_banner -loglevel error -y -i "$SRC_Y4M" \ + -c:v libsvtav1 -preset 8 $mode "$OUT/det.$i.ivf" 2>/dev/null + done + if cmp -s "$OUT/det.1.ivf" "$OUT/det.2.ivf"; then + pass "baseline reproducible with '$mode'" + else + printf ' \033[33mNOTE\033[0m baseline NOT reproducible with '"'"'%s'"'"' (SVT-AV1 property, unrelated to the patch)\n' "$mode" + fi +done +# low delay: the guarded path, must behave exactly as before and must not hang +run_case lowdelay "$SRC_Y4M" -c:v libsvtav1 -preset 8 -crf 40 \ + -svtav1-params pred-struct=1 +# rtc forces low delay INSIDE the library (enc_handle.c:4317) without updating +# the configuration the wrapper holds: an early poll that trusts pred_structure +# alone blocks here forever. Regression test for exactly that. +run_case rtc "$SRC_Y4M" -c:v libsvtav1 -preset 10 -crf 40 \ + -svtav1-params rtc=1 +run_case allintra "$SRC_Y4M" -c:v libsvtav1 -preset 10 -crf 40 \ + -svtav1-params pred-struct=0 +run_case lp1 "$SRC_Y4M" -c:v libsvtav1 -preset 8 -crf 40 -svtav1-params lp=1 +run_case transcode "$SRC_MP4" -c:v libsvtav1 -preset 10 -crf 40 -g 120 + +# Real-world content, if a clip was mounted at /media/clip.* +REAL=$(ls /media/clip.* 2>/dev/null | head -1) +if [ -n "$REAL" ]; then + hdr "real content: $REAL" + "$PATCHED/ffprobe" -hide_banner -loglevel error -select_streams v:0 \ + -show_entries stream=codec_name,width,height,r_frame_rate,nb_frames \ + -show_entries format=duration,size -of default=nw=1 "$REAL" | sed 's/^/ /' + run_case real-p8 "$REAL" -c:v libsvtav1 -preset 8 -crf 40 -g 120 -an + run_case real-hier "$REAL" -c:v libsvtav1 -preset 6 -crf 35 -g 240 -an \ + -svtav1-params hierarchical-levels=5:lookahead=120 +fi + +# ------------------------------------------------------- behavioural proof --- +hdr "drain behaviour (libavcodec API only, 300 frames)" +for v in base patched; do + bin=$BASE; [ "$v" = patched ] && bin=$PATCHED + echo "-- $v" + timeout -k 5 "$TMO" "$bin/drain_probe" 300 8 "hierarchical-levels=5" \ + > "$OUT/probe.$v.txt" 2>&1 + rc=$? + sed 's/^/ /' "$OUT/probe.$v.txt" + [ $rc -eq 0 ] || fail "drain_probe/$v exited $rc" +done + +maxb=$(awk '/max_packets_per_send/{print $2}' "$OUT/probe.base.txt") +maxp=$(awk '/max_packets_per_send/{print $2}' "$OUT/probe.patched.txt") +mrp=$(awk '/rounds_with_multi_pkt/{print $2}' "$OUT/probe.patched.txt") +pk_b=$(awk '/^packets_received/{print $2}' "$OUT/probe.base.txt") +pk_p=$(awk '/^packets_received/{print $2}' "$OUT/probe.patched.txt") + +[ "${maxb:-0}" = 1 ] \ + && pass "baseline caps at 1 packet per send_frame (as analysed)" \ + || fail "baseline reported max_packets_per_send=${maxb:-?}, expected 1" +[ "${maxp:-0}" -gt 1 ] 2>/dev/null \ + && pass "patched drains bursts: max ${maxp} packets/send, ${mrp} multi-packet rounds" \ + || fail "patched reported max_packets_per_send=${maxp:-?}, expected > 1" +[ "${pk_b:-0}" = "${pk_p:-1}" ] \ + && pass "same total packet count ($pk_b)" \ + || fail "packet count differs: base $pk_b vs patched $pk_p" + +# ----------------------------------------------------- pool pressure stress -- +# Smallest pools (lp=1 shrinks picture_control_set_pool_init_count, and with it +# output_stream_buffer_fifo_init_count = pcs_pool + 2) combined with the deepest +# hierarchy, i.e. the largest output bursts. This is the configuration where the +# one-packet-per-frame cap is most likely to pin the output pool at its ceiling. +hdr "pool pressure stress (lp=1, hierarchical-levels=5, ${STRESS_FRAMES:-1500} frames)" +for v in base patched; do + bin=$BASE; [ "$v" = patched ] && bin=$PATCHED + start=$SECONDS + timeout -k 5 "$TMO" "$bin/drain_probe" "${STRESS_FRAMES:-1500}" 10 \ + "lp=1:hierarchical-levels=5:lookahead=120" > "$OUT/stress.$v.txt" 2>&1 + rc=$? + el=$((SECONDS-start)) + if [ $rc -eq 124 ] || [ $rc -eq 137 ]; then + printf ' \033[33mNOTE\033[0m stress/%s TIMED OUT after %ss — reproduces the stall\n' "$v" "$TMO" + [ "$v" = patched ] && fail "patched build stalled under stress" + elif [ $rc -ne 0 ]; then + fail "stress/$v exited $rc" + else + pass "stress/$v completed in ${el}s" + fi + grep -E 'frames_sent|packets_received|max_packets_per_send|rounds_with_multi_pkt|flush_round' \ + "$OUT/stress.$v.txt" | sed 's/^/ /' +done + +# ------------------------------------------------------------------ summary -- +hdr "summary" +if [ $fails -eq 0 ]; then + printf ' \033[32mall checks passed\033[0m (results in %s)\n' "$OUT" +else + printf ' \033[31m%d check(s) failed\033[0m (results in %s)\n' "$fails" "$OUT" +fi +exit $((fails > 0)) -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
