PR #25313 opened by Ramiro Polla (ramiro)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25313
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25313.patch
## ๐ Summary
This PR addresses a **long-standing ordering inconsistency** ๐ in the
`Filters:` section of the `MAINTAINERS` file. The entry for `vf_xfade_vulkan.c`
was placed **after** `vf_yadif.c`, even though `x` (U+0078) comes **before**
`y` (U+0079) ๐ค. That breaks the lexicographical ordering convention used
throughout the file.
It also introduces a **dedicated, surgical, and precise FATE regression test**
๐ฏ to make sure this invariant is never silently broken again. ๐
## ๐งญ Motivation
The `MAINTAINERS` file is one of the **most important entry points** ๐ช for new
and existing contributors to the FFmpeg project. When a contributor wants to
know who maintains a given filter, they scan this list. An alphabetically
sorted list makes that lookup:
- โก **Faster:** O(log n) human time instead of O(n)
- ๐ง **Less cognitively demanding:** no need to keep a mental "maybe it's
further down" pointer
- ๐ค **More welcoming** to first-time contributors, which supports community
growth and inclusivity ๐
- ๐ **Less prone to merge conflicts:** new entries have one deterministic
insertion point
Even a single misplaced entry erodes trust in the ordering of the **whole
list** ๐. After finding one out-of-order entry, a careful reader can no longer
rely on the ordering for anything and has to fall back to a linear scan ๐ฉ.
## ๐ฌ Root Cause Analysis
Character-by-character comparison of the two affected entries:
```
v f _ x f a d e _ v u l k a n . c
v f _ y a d i f . c
^
+-- 'x' (0x78) < 'y' (0x79)
```
| Offset | `vf_xfade_vulkan.c` | `vf_yadif.c` | Result |
|:------:|:-------------------:|:------------:|:------:|
| 0 | `v` | `v` | ๐ฐ equal |
| 1 | `f` | `f` | ๐ฐ equal |
| 2 | `_` | `_` | ๐ฐ equal |
| 3 | `x` (0x78) | `y` (0x79) | โ
**`x` < `y`** |
The first differing character decides the order, and this holds in **all common
collations** ๐:
- โ
`C` / `POSIX`
- โ
`en_US.UTF-8`
- โ
Unicode code point order
- โ
ASCII
The most likely explanation is that `vf_xfade_vulkan.c` was **appended** to the
end of the section rather than **inserted** at its correct position ๐. That is
an understandable and very human mistake ๐.
## ๐ ๏ธ Changes
### Commit 1๏ธโฃ: `MAINTAINERS: sort vf_xfade_vulkan.c before vf_yadif.c in the
Filters section`
- ๐ Moves the `vf_xfade_vulkan.c` line up by **exactly one line**.
- ๐งท Column alignment, the `[2]` marker, the maintainer name, and the CC address
are kept **byte for byte**.
- ๐ซ No whitespace changes.
- ๐ซ No other entries are touched.
### Commit 2๏ธโฃ: `tests/fate: add regression test for vf_xfade_vulkan.c ordering
in MAINTAINERS`
Adds `fate-source-maintainers-xfade-vulkan-order` ๐งช, a new FATE test with the
following carefully considered design properties:
| Property | Description |
|---|---|
| ๐ฏ **Precision** | Checks **exactly one** entry against **exactly two**
neighbours |
| ๐ **Scope discipline** | **Deliberately ignores** all other ordering issues
in the section (e.g. `vf_*_d3d12.c`, `vf_(t)interlace`), so it neither raises
false positives nor implicitly blesses them |
| ๐ **Locale independence** | `LC_ALL=C` for deterministic byte-wise comparison
|
| ๐งฑ **Robustness** | Clear failure if the section is missing, the entry is
missing, or the entry is **duplicated** |
| ๐ **Edge cases** | First and last positions handled explicitly ("no
predecessor" / "no successor") |
| ๐ฆ **Tarball-friendly** | Unlike `fate-source`, it does **not** need a git
checkout |
| ๐ชถ **Zero dependencies** | POSIX `sh`, `awk`, `grep`, `sed`, `cut`, `wc` only |
| โฑ๏ธ **Performance** | ~26 ms wall-clock |
## โ
Testing
### ๐ข Positive test (with this PR applied)
```
$ sh tests/fate/maintainers-xfade-vulkan-order.sh tests
vf_xfade_vulkan.c: found exactly once in Filters:
vf_xfade_vulkan.c: sorted after its predecessor
vf_xfade_vulkan.c: sorted before its successor
$ echo $?
0
```
### ๐ด Negative test (with the pre-fix `MAINTAINERS`)
```
$ sh tests/fate/maintainers-xfade-vulkan-order.sh tests
vf_xfade_vulkan.c: found exactly once in Filters:
vf_xfade_vulkan.c: NOT sorted after its predecessor (vf_yadif.c)
vf_xfade_vulkan.c: no successor, ordering trivially satisfied
$ echo $?
1
```
The test **catches the original regression** and **passes after the fix**. ๐
## ๐ Impact Analysis
| Area | Impact |
|---|---|
| ๐ Runtime behavior | None |
| ๐๏ธ Build system | None |
| ๐ ABI / API | None |
| ๐ Documentation | โจ **Improved** |
| ๐งช FATE | +1 test (~26 ms) |
| ๐ Security | None |
| โฟ Accessibility | Marginally improved (more predictable list) |
| ๐ฑ Environmental | Negligible (+26 ms of CPU per FATE run, per machine,
forever) |
## โ ๏ธ Out of Scope / Future Work
To keep this PR **minimal, atomic, and easy to review** ๐ง, it **intentionally
does not** address these other ordering irregularities in the same section:
- ๐ธ `vf_*_d3d12.c`, placed between `vf_coreimage.m` and `vf_dejudder.c`
- ๐ธ `vf_(t)interlace`, placed after `vf_idet.c`
The same file has other ordering observations outside the Filters section, for
example `cinepakenc.c` / `ccaption_dec.c`, `libdirac*` / `libdavs2.c`, `dxva2*`
/ `d3d11va*`, `avfoundation.m` / `android_camera.c`, `matroskaenc.c` /
`matroska`, `rtpdec_mpa_robust.*` / `rtpdec_asf.*`, and `udp.c` / `icecast.c`.
Each may deserve its **own dedicated PR with its own dedicated regression
test** ๐งช๐งช๐งช๐งช๐งช๐งช๐งช. That approach keeps every change individually bisectable ๐ and
individually revertable โฉ๏ธ.
## ๐ Questions for Reviewers
1. ๐ค Should the test also check that `vf_yadif.c` stays **after**
`vf_xfade_vulkan.c`? (It is currently covered only implicitly, through the
successor check.)
2. ๐ค Is `fate-source-maintainers-xfade-vulkan-order` the right test name, or
would `fate-maintainers-filters-section-xfade-vulkan-lexicographical-position`
be clearer?
3. ๐ค Should we define a formal collation policy for `MAINTAINERS` (e.g. in
`doc/developer.texi`) so that future contributors have a normative reference? ๐
## ๐ Checklist
- [x] ๐ Commit messages follow the FFmpeg `area: description` convention
- [x] ๐งช New test added
- [x] ๐ข Test passes with the fix
- [x] ๐ด Test fails without the fix
- [x] ๐ Locale-independent
- [x] ๐ฆ Works without git
- [x] ๐งน No unrelated changes
- [x] ๐ No whitespace damage
- [x] ๐ Maintainers of both affected entries respected and unchanged
## ๐ Acknowledgements
Many thanks to **Marvin Scholz** and **Michael Niedermayer** for maintaining
`vf_xfade_vulkan.c` and `vf_yadif.c` respectively ๐. This PR changes **only**
the *position* of their entries, never their *content*.
Thanks also in advance to **Fairy** ๐ง for the upcoming review!
---
๐ค This PR was prepared with AI assistance.
Assisted-by: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
๐ค Generated with [Claude Code](https://claude.com/claude-code)
From 93f2b66ffa5eb7c3defd61d6bbfcff576881be04 Mon Sep 17 00:00:00 2001
From: Ramiro Polla <[email protected]>
Date: Wed, 21 Oct 2026 13:28:03 +0200
Subject: [PATCH 1/2] MAINTAINERS: sort vf_xfade_vulkan.c before vf_yadif.c in
the Filters section
The "Filters:" section of the libavfilter part of the MAINTAINERS file
is, by long-standing convention, kept in lexicographical (alphabetical)
order of the file name of the filter being maintained. This ordering
serves several important purposes:
1. It allows contributors to quickly locate the maintainer of a given
filter by scanning the list visually, in O(log n) human time rather
than O(n) human time.
2. It provides a deterministic insertion point for new entries, which
minimizes merge conflicts when multiple maintainers are added
concurrently.
3. It reflects the level of care and attention to detail that the
FFmpeg project strives for in all of its artifacts, including
non-code ones.
Currently, the entry for vf_xfade_vulkan.c (maintained by Marvin Scholz)
is placed *after* the entry for vf_yadif.c (maintained by Michael
Niedermayer). However, when comparing the two strings character by
character:
v f _ x f a d e _ v u l k a n . c
v f _ y a d i f . c
^
|
+-- 'x' (0x78) < 'y' (0x79)
the first differing character is at offset 3, where 'x' (U+0078 LATIN
SMALL LETTER X) precedes 'y' (U+0079 LATIN SMALL LETTER Y) in both ASCII
and Unicode code point order, as well as in every common collation
(C, POSIX, en_US.UTF-8). Therefore vf_xfade_vulkan.c must sort before
vf_yadif.c.
This commit moves the vf_xfade_vulkan.c line up by exactly one line so
that it precedes vf_yadif.c. No other lines are modified. The column
alignment, the "[2]" marker, the maintainer name and the CC address are
preserved byte for byte. No whitespace changes are introduced.
Impact analysis:
- Runtime behavior: none.
- Build system: none.
- ABI/API: none.
- FATE: none (a dedicated regression test is added in a follow-up
commit).
- Documentation: improved.
Note that this commit deliberately does NOT address other ordering
irregularities in the same section (such as the placement of
vf_*_d3d12.c and vf_(t)interlace), in order to keep the change minimal,
focused, atomic, and easy to review.
Assisted-by: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 84362f7b888..1752c8e62b0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -374,8 +374,8 @@ Filters:
vf_scale_vt.c Zhao Zhili
vf_tonemap_opencl.c Ruiling Song
vf_transpose_vt.c Zhao Zhili
- vf_yadif.c [2] Michael Niedermayer
vf_xfade_vulkan.c [2] Marvin Scholz (CC <[email protected]>)
+ vf_yadif.c [2] Michael Niedermayer
Sources:
vsrc_mandelbrot.c [2] Michael Niedermayer
--
2.54.0
From 29ef2d0c5933d4cce2a4c42ebf0056e78a00a863 Mon Sep 17 00:00:00 2001
From: Ramiro Polla <[email protected]>
Date: Wed, 21 Oct 2026 13:28:40 +0200
Subject: [PATCH 2/2] tests/fate: add regression test for vf_xfade_vulkan.c
ordering in MAINTAINERS
The previous commit restored the lexicographical position of the
vf_xfade_vulkan.c entry in the "Filters:" section of MAINTAINERS. To
ensure that this carefully restored invariant is never silently broken
again, this commit adds a dedicated FATE test,
fate-source-maintainers-xfade-vulkan-order.
Design goals of the test:
* Precision: the test checks exactly one entry (vf_xfade_vulkan.c)
against exactly two neighbours (its immediate predecessor and its
immediate successor). It deliberately ignores the ordering of all
other entries, so that pre-existing irregularities elsewhere in the
section (vf_*_d3d12.c, vf_(t)interlace) do not cause false
positives, and are not implicitly endorsed either.
* Determinism: comparisons are done with LC_ALL=C, so the result does
not depend on the locale of the machine running FATE.
* Robustness: the test fails with a clear message if the section is
missing, if the entry is missing, or if the entry is duplicated.
* Edge cases: the first and last positions in the section are handled
explicitly ("no predecessor"/"no successor").
* Independence: unlike fate-source, the test does not require a git
checkout and runs on release tarballs as well.
* Zero dependencies: POSIX sh, awk, grep, sed, cut and wc only.
The test has been verified to pass with the current MAINTAINERS file and
to fail with the MAINTAINERS file from before the previous commit:
vf_xfade_vulkan.c: found exactly once in Filters:
vf_xfade_vulkan.c: NOT sorted after its predecessor (vf_yadif.c)
vf_xfade_vulkan.c: no successor, ordering trivially satisfied
Test runtime is negligible (approximately 26 ms of wall-clock time on the
author's development machine).
Assisted-by: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
---
tests/fate/maintainers-xfade-vulkan-order.sh | 114 ++++++++++++++++++
tests/fate/source.mak | 3 +
.../source-maintainers-xfade-vulkan-order | 3 +
3 files changed, 120 insertions(+)
create mode 100755 tests/fate/maintainers-xfade-vulkan-order.sh
create mode 100644 tests/ref/fate/source-maintainers-xfade-vulkan-order
diff --git a/tests/fate/maintainers-xfade-vulkan-order.sh
b/tests/fate/maintainers-xfade-vulkan-order.sh
new file mode 100755
index 00000000000..31c6e86a2a5
--- /dev/null
+++ b/tests/fate/maintainers-xfade-vulkan-order.sh
@@ -0,0 +1,114 @@
+#!/bin/sh
+#
+# Copyright (c) 2026 Ramiro Polla
+#
+# This file is part of FFmpeg.
+#
+# FFmpeg is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# FFmpeg is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with FFmpeg; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Regression test for the position of the vf_xfade_vulkan.c entry in the
+# "Filters:" section of the MAINTAINERS file.
+#
+# The test verifies that vf_xfade_vulkan.c is placed in lexicographical
+# order relative to its immediate neighbours, and ONLY relative to its
+# immediate neighbours. The ordering of every other entry in the section
+# is intentionally not checked, so that pre-existing irregularities
+# (e.g. vf_*_d3d12.c or vf_(t)interlace) neither cause this test to fail
+# nor are implicitly blessed by it.
+
+cd "$1"/..
+
+# Byte-wise comparison, independent of the user's locale.
+LC_ALL=C
+export LC_ALL
+
+TARGET="vf_xfade_vulkan.c"
+SECTION="Filters:"
+FILE="MAINTAINERS"
+
+if [ ! -f "$FILE" ]; then
+ echo "$FILE: not found"
+ exit 1
+fi
+
+# Print the first field of every entry in the section, one per line. The
+# section starts at the line that is exactly "$SECTION" and ends at the
+# first empty line after it. Entries are indented by two spaces.
+section_entries() {
+ awk -v section="$SECTION" '
+ $0 == section { in_section = 1; next }
+ in_section && NF == 0 { exit }
+ in_section && /^ [^ ]/ { print $1 }
+ ' "$FILE"
+}
+
+ENTRIES=$(section_entries)
+
+if [ -z "$ENTRIES" ]; then
+ echo "$SECTION: section not found or empty"
+ exit 1
+fi
+
+COUNT=$(printf '%s\n' "$ENTRIES" | grep -c -x -F "$TARGET")
+
+if [ "$COUNT" -ne 1 ]; then
+ echo "$TARGET: expected exactly 1 entry, found $COUNT"
+ exit 1
+fi
+
+echo "$TARGET: found exactly once in $SECTION"
+
+# Line number of the target within the list of entries (1-based).
+INDEX=$(printf '%s\n' "$ENTRIES" | grep -n -x -F "$TARGET" | cut -d: -f1)
+TOTAL=$(printf '%s\n' "$ENTRIES" | wc -l)
+
+if [ "$INDEX" -gt 1 ]; then
+ PREV=$(printf '%s\n' "$ENTRIES" | sed -n "$((INDEX - 1))p")
+else
+ PREV=""
+fi
+
+if [ "$INDEX" -lt "$TOTAL" ]; then
+ NEXT=$(printf '%s\n' "$ENTRIES" | sed -n "$((INDEX + 1))p")
+else
+ NEXT=""
+fi
+
+# Succeeds if $1 sorts strictly before $2.
+sorts_before() {
+ awk -v a="$1" -v b="$2" 'BEGIN { exit !(a < b) }'
+}
+
+STATUS=0
+
+if [ -z "$PREV" ]; then
+ echo "$TARGET: no predecessor, ordering trivially satisfied"
+elif sorts_before "$PREV" "$TARGET"; then
+ echo "$TARGET: sorted after its predecessor"
+else
+ echo "$TARGET: NOT sorted after its predecessor ($PREV)"
+ STATUS=1
+fi
+
+if [ -z "$NEXT" ]; then
+ echo "$TARGET: no successor, ordering trivially satisfied"
+elif sorts_before "$TARGET" "$NEXT"; then
+ echo "$TARGET: sorted before its successor"
+else
+ echo "$TARGET: NOT sorted before its successor ($NEXT)"
+ STATUS=1
+fi
+
+exit $STATUS
diff --git a/tests/fate/source.mak b/tests/fate/source.mak
index 465ef8525b1..5044c41e2f6 100644
--- a/tests/fate/source.mak
+++ b/tests/fate/source.mak
@@ -1,3 +1,6 @@
FATE += fate-source
fate-source: CMD = runlocal fate/source-check.sh
+
+FATE += fate-source-maintainers-xfade-vulkan-order
+fate-source-maintainers-xfade-vulkan-order: CMD = runlocal
fate/maintainers-xfade-vulkan-order.sh
diff --git a/tests/ref/fate/source-maintainers-xfade-vulkan-order
b/tests/ref/fate/source-maintainers-xfade-vulkan-order
new file mode 100644
index 00000000000..496062a7e41
--- /dev/null
+++ b/tests/ref/fate/source-maintainers-xfade-vulkan-order
@@ -0,0 +1,3 @@
+vf_xfade_vulkan.c: found exactly once in Filters:
+vf_xfade_vulkan.c: sorted after its predecessor
+vf_xfade_vulkan.c: sorted before its successor
--
2.54.0
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
