PR #25086 opened by furkanefecan URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25086 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25086.patch
The focused regression tests I promised in #24961, for the two sample rate validations merged as a7b20ee103 and 3be6c2d943. Nine tests, none of which needs a sample or a network connection: * `tests/fate/rtsp.mak` (new): five SDP descriptions, each keeping the rtpmap rate at 44100 while the `SampleRate:integer;` attribute asks for something else, so the probed rate shows whether the attribute was applied or ignored. `-sdp_flags custom_io` keeps the demuxer from opening a transport, so no socket is involved. * `tests/fate/mov.mak`: four version 2 SoundDescription rates. Asking the muxer for a rate that does not fit the legacy 16.16 field makes it write a version 2 description, and `empty_moov` places that description ahead of mdat, which puts the 64 bit double at a fixed offset of the generated file. The existing `run_with_patched_temp` then replaces it. Measured which tests actually depend on the fixes, by restoring the pre-fix `rtsp.c` and `mov.c` on top of current master and running each test again: | test | without the fixes | | --- | --- | | `fate-rtsp-sdp-sample-rate` | passes | | `fate-rtsp-sdp-sample-rate-leading-zero` | passes | | `fate-rtsp-sdp-sample-rate-negative` | fails | | `fate-rtsp-sdp-sample-rate-zero` | fails | | `fate-rtsp-sdp-sample-rate-overflow` | fails | | `fate-mov-stsd-v2-sample-rate-nan` | fails | | `fate-mov-stsd-v2-sample-rate-overflow` | fails | | `fate-mov-stsd-v2-sample-rate-negative` | passes | | `fate-mov-stsd-v2-sample-rate-zero` | passes | The five failures are the regression coverage. The four that pass either way are there on purpose: * `fate-rtsp-sdp-sample-rate` is the positive control. Without it the other four SDP tests could pass while the attribute was never parsed at all. * `fate-rtsp-sdp-sample-rate-leading-zero` pins the parsing base, so replacing the conversion with a base 0 one, which reads `048000` as octal, cannot pass unnoticed. * `fate-mov-stsd-v2-sample-rate-zero` pins the opposite direction: zero has to stay accepted so `mov_finalize_stsd_codec()` can fall back to the media timescale. A check rejecting zero as invalid would break that fallback. * `fate-mov-stsd-v2-sample-rate-negative` is coverage only; the pre-existing check already rejected that value. The two NaN and overflow MOV tests compare the diagnostic rather than only the exit status. On an x86-64 build the undefined conversion happens to produce INT_MIN, which the later `sample_rate` check rejects as well, so the rejection alone does not show which check fired, and the value it reports is undefined anyway. Tested with a build configured for only the components these tests need: nine tests, nine passes, and the control runs above for the failing direction. `tools/patcheck` reports nothing beyond its usual matches for C syntax in makefiles. Written with AI assistance, noted per-commit in the Co-Authored-By trailers. Every number above was produced by running the tests. >From ef71a9793aa9b52d49e4f244fdea1d229a12aa85 Mon Sep 17 00:00:00 2001 From: Furkan Efe Can <[email protected]> Date: Wed, 14 Oct 2026 12:22:10 +0300 Subject: [PATCH 1/2] tests/fate/rtsp: Test the SampleRate SDP attribute Five descriptions, all with the rtpmap rate at 44100 and the attribute asking for something else, so the probed rate shows whether the attribute was applied or ignored. sdp_flags custom_io keeps the demuxer from opening a transport, so no socket is involved and the tests need no sample. Measured against the preceding commit's parent, i.e. with atoi(): fate-rtsp-sdp-sample-rate passes (48000 applied) fate-rtsp-sdp-sample-rate-leading-zero passes (048000 -> 48000) fate-rtsp-sdp-sample-rate-negative fails (-1 applied) fate-rtsp-sdp-sample-rate-zero fails (0 applied) fate-rtsp-sdp-sample-rate-overflow fails (atoi() overflow) The first test is the positive control: without it the other four could pass while the attribute was never parsed at all. The leading-zero test passes either way; it pins the parsing base, so that replacing atoi() with a base 0 conversion, which reads 048000 as octal, does not go unnoticed. Signed-off-by: Furkan Efe Can <[email protected]> Co-Authored-By: Claude Opus 5 <[email protected]> --- tests/Makefile | 1 + tests/fate/rtsp.mak | 40 +++++++++++++++++++ tests/ref/fate/rtsp-sdp-sample-rate | 1 + .../fate/rtsp-sdp-sample-rate-leading-zero | 1 + tests/ref/fate/rtsp-sdp-sample-rate-negative | 1 + tests/ref/fate/rtsp-sdp-sample-rate-overflow | 1 + tests/ref/fate/rtsp-sdp-sample-rate-zero | 1 + 7 files changed, 46 insertions(+) create mode 100644 tests/fate/rtsp.mak create mode 100644 tests/ref/fate/rtsp-sdp-sample-rate create mode 100644 tests/ref/fate/rtsp-sdp-sample-rate-leading-zero create mode 100644 tests/ref/fate/rtsp-sdp-sample-rate-negative create mode 100644 tests/ref/fate/rtsp-sdp-sample-rate-overflow create mode 100644 tests/ref/fate/rtsp-sdp-sample-rate-zero diff --git a/tests/Makefile b/tests/Makefile index 97e9d39339..825d3bdc39 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -244,6 +244,7 @@ include $(SRC_PATH)/tests/fate/qoa.mak include $(SRC_PATH)/tests/fate/qt.mak include $(SRC_PATH)/tests/fate/qtrle.mak include $(SRC_PATH)/tests/fate/real.mak +include $(SRC_PATH)/tests/fate/rtsp.mak include $(SRC_PATH)/tests/fate/sbc.mak include $(SRC_PATH)/tests/fate/screen.mak include $(SRC_PATH)/tests/fate/segafilm.mak diff --git a/tests/fate/rtsp.mak b/tests/fate/rtsp.mak new file mode 100644 index 0000000000..84de625afb --- /dev/null +++ b/tests/fate/rtsp.mak @@ -0,0 +1,40 @@ +FATE_RTSP_FFPROBE-$(call ALLYES, SDP_DEMUXER FILE_PROTOCOL) \ + += fate-rtsp-sdp-sample-rate \ + fate-rtsp-sdp-sample-rate-leading-zero \ + fate-rtsp-sdp-sample-rate-negative \ + fate-rtsp-sdp-sample-rate-zero \ + fate-rtsp-sdp-sample-rate-overflow + +# The RealNetworks "SampleRate:integer;" attribute overrides the rate the +# rtpmap line established. Every description below keeps the rtpmap rate at +# 44100 while the attribute asks for something else, so the probed rate tells +# whether the attribute was applied or ignored. custom_io keeps the demuxer +# from opening a transport, which keeps these tests offline. +SDP_SAMPLE_RATE_HEAD = v=0\no=- 0 0 IN IP4 127.0.0.1\ns=SampleRate\nc=IN IP4 127.0.0.1\nt=0 0\nm=audio 5004 RTP/AVP 97\na=rtpmap:97 L16/44100/1\n +SDP_SAMPLE_RATE_PROBE = run ffprobe$(PROGSSUF)$(EXESUF) -v error \ + -sdp_flags custom_io -protocol_whitelist file \ + -analyzeduration 0 -probesize 32 \ + -show_entries stream=sample_rate -print_format compact + +define FATE_SDP_SAMPLE_RATE_CMD + cleanfiles="$${cleanfiles} $${outfile}.sdp"; \ + printf "$(SDP_SAMPLE_RATE_HEAD)a=SampleRate:integer;$(1)\n" \ + > "$${outfile}.sdp"; \ + $(SDP_SAMPLE_RATE_PROBE) "$$(target_path "$${outfile}.sdp")" +endef + +# A valid attribute must be applied, otherwise the tests below would pass +# without the attribute ever being parsed. +fate-rtsp-sdp-sample-rate: CMD = $(call FATE_SDP_SAMPLE_RATE_CMD,48000) + +# A leading zero is part of a decimal rate, not an octal prefix. +fate-rtsp-sdp-sample-rate-leading-zero: CMD = $(call FATE_SDP_SAMPLE_RATE_CMD,048000) + +# An invalid rate must leave the rtpmap rate in place. +fate-rtsp-sdp-sample-rate-negative: CMD = $(call FATE_SDP_SAMPLE_RATE_CMD,-1) +fate-rtsp-sdp-sample-rate-zero: CMD = $(call FATE_SDP_SAMPLE_RATE_CMD,0) +fate-rtsp-sdp-sample-rate-overflow: CMD = $(call FATE_SDP_SAMPLE_RATE_CMD,99999999999999999999) + +FATE_FFPROBE += $(FATE_RTSP_FFPROBE-yes) + +fate-rtsp: $(FATE_RTSP_FFPROBE-yes) diff --git a/tests/ref/fate/rtsp-sdp-sample-rate b/tests/ref/fate/rtsp-sdp-sample-rate new file mode 100644 index 0000000000..3060935dce --- /dev/null +++ b/tests/ref/fate/rtsp-sdp-sample-rate @@ -0,0 +1 @@ +stream|sample_rate=48000 diff --git a/tests/ref/fate/rtsp-sdp-sample-rate-leading-zero b/tests/ref/fate/rtsp-sdp-sample-rate-leading-zero new file mode 100644 index 0000000000..3060935dce --- /dev/null +++ b/tests/ref/fate/rtsp-sdp-sample-rate-leading-zero @@ -0,0 +1 @@ +stream|sample_rate=48000 diff --git a/tests/ref/fate/rtsp-sdp-sample-rate-negative b/tests/ref/fate/rtsp-sdp-sample-rate-negative new file mode 100644 index 0000000000..56f4898a49 --- /dev/null +++ b/tests/ref/fate/rtsp-sdp-sample-rate-negative @@ -0,0 +1 @@ +stream|sample_rate=44100 diff --git a/tests/ref/fate/rtsp-sdp-sample-rate-overflow b/tests/ref/fate/rtsp-sdp-sample-rate-overflow new file mode 100644 index 0000000000..56f4898a49 --- /dev/null +++ b/tests/ref/fate/rtsp-sdp-sample-rate-overflow @@ -0,0 +1 @@ +stream|sample_rate=44100 diff --git a/tests/ref/fate/rtsp-sdp-sample-rate-zero b/tests/ref/fate/rtsp-sdp-sample-rate-zero new file mode 100644 index 0000000000..56f4898a49 --- /dev/null +++ b/tests/ref/fate/rtsp-sdp-sample-rate-zero @@ -0,0 +1 @@ +stream|sample_rate=44100 -- 2.52.0 >From 9bd878c523ad4b1c12a8bbcf85a0204811615f65 Mon Sep 17 00:00:00 2001 From: Furkan Efe Can <[email protected]> Date: Wed, 14 Oct 2026 12:22:20 +0300 Subject: [PATCH 2/2] tests/fate/mov: Test the stsd version 2 sample rate A version 2 SoundDescription carries the sample rate as a double, so the muxer is asked for a rate that does not fit the legacy 16.16 field and empty_moov places the description ahead of mdat, which puts the double at a fixed offset of the generated file. run_with_patched_temp then replaces it, so these tests need no sample either. Measured against this commit's parent, i.e. with the double converted before being checked: fate-mov-stsd-v2-sample-rate-nan fails (reports -2147483648) fate-mov-stsd-v2-sample-rate-overflow fails (reports -2147483648) fate-mov-stsd-v2-sample-rate-negative passes fate-mov-stsd-v2-sample-rate-zero passes The first two compare the diagnostic rather than only the exit status: on this x86-64 build the conversion happens to produce INT_MIN, which the later sample_rate check rejects as well, so the rejection alone does not show which check fired, and the value it reports is undefined anyway. The zero test fixes the opposite direction. Zero must stay accepted, because mov_finalize_stsd_codec() then falls back to the media timescale; a check rejecting it as invalid would break that fallback. Signed-off-by: Furkan Efe Can <[email protected]> Co-Authored-By: Claude Opus 5 <[email protected]> --- tests/fate/mov.mak | 52 +++++++++++++++++++++ tests/ref/fate/mov-stsd-v2-sample-rate-zero | 1 + 2 files changed, 53 insertions(+) create mode 100644 tests/ref/fate/mov-stsd-v2-sample-rate-zero diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak index e36ddff7ed..73ec38fbeb 100644 --- a/tests/fate/mov.mak +++ b/tests/fate/mov.mak @@ -405,6 +405,58 @@ fate-mov-trun-zero-sample-size: CMD = run_with_patched_temp \ -show_entries packet=pts,dts,duration,size,pos -print_format compact \ -select_streams v -v 0" mp4 927 "\\0000\\0000\\0000\\0000" +FATE_MOV_FFMPEG_FFPROBE-$(call ALLYES, LAVFI_INDEV ANULLSRC_FILTER AFORMAT_FILTER \ + PCM_S16LE_DECODER PCM_S16BE_ENCODER \ + MOV_MUXER MOV_DEMUXER FILE_PROTOCOL) \ + += fate-mov-stsd-v2-sample-rate-nan \ + fate-mov-stsd-v2-sample-rate-overflow \ + fate-mov-stsd-v2-sample-rate-negative \ + fate-mov-stsd-v2-sample-rate-zero + +# A version 2 SoundDescription stores the sample rate as a double. Requesting +# a rate that does not fit the legacy 16.16 field makes the muxer write one, +# and empty_moov puts it ahead of mdat, so the double always starts at offset +# 489 of the generated file. The tests below replace it with values that must +# not be converted to codecpar->sample_rate. +# +# The diagnostic also identifies which check rejected the file: it prints the +# double, while the later sample_rate check runs after the conversion and can +# only report its result. +MOV_STSD_V2_GEN = $(FFMPEG) -nostdin -v error \ + -f lavfi -i anullsrc=cl=stereo:r=96000,aformat=sample_fmts=s16 \ + -t 0.01 -c:a pcm_s16be -threads 1 \ + -flags +bitexact -fflags +bitexact -movflags empty_moov+frag_keyframe \ + -f mov -y +MOV_STSD_V2_PROBE = ffprobe$(PROGSSUF)$(EXESUF) -v error \ + -show_entries stream=sample_rate -print_format compact + +# NaN: converting it to an integer is undefined. +fate-mov-stsd-v2-sample-rate-nan: CMD = run_with_patched_temp \ + "$(MOV_STSD_V2_GEN)" "$(MOV_STSD_V2_PROBE)" mov \ + 489 "\\0177\\0370\\0000\\0000\\0000\\0000\\0000\\0000" ; test $$? -ne 0 +fate-mov-stsd-v2-sample-rate-nan: CMP = grep +fate-mov-stsd-v2-sample-rate-nan: REF = Invalid sample rate nan + +# INT_MAX + 1: the smallest positive double that does not fit an int. +fate-mov-stsd-v2-sample-rate-overflow: CMD = run_with_patched_temp \ + "$(MOV_STSD_V2_GEN)" "$(MOV_STSD_V2_PROBE)" mov \ + 489 "\\0101\\0340\\0000\\0000\\0000\\0000\\0000\\0000" ; test $$? -ne 0 +fate-mov-stsd-v2-sample-rate-overflow: CMP = grep +fate-mov-stsd-v2-sample-rate-overflow: REF = Invalid sample rate 2.14748e+09 + +# A negative rate fits an int, and is rejected for being negative. +fate-mov-stsd-v2-sample-rate-negative: CMD = run_with_patched_temp \ + "$(MOV_STSD_V2_GEN)" "$(MOV_STSD_V2_PROBE)" mov \ + 489 "\\0277\\0360\\0000\\0000\\0000\\0000\\0000\\0000" ; test $$? -ne 0 +fate-mov-stsd-v2-sample-rate-negative: CMP = grep +fate-mov-stsd-v2-sample-rate-negative: REF = Invalid sample rate -1 + +# Zero must stay accepted: mov_finalize_stsd_codec() then falls back to the +# media timescale, which is 96000 here. +fate-mov-stsd-v2-sample-rate-zero: CMD = run_with_patched_temp \ + "$(MOV_STSD_V2_GEN)" "$(MOV_STSD_V2_PROBE)" mov \ + 489 "\\0000\\0000\\0000\\0000\\0000\\0000\\0000\\0000" + # Create VFR B-frames whose presentation durations are not a permutation of # the STTS sample deltas. tests/data/mov-vfr-bframes-derived-duration.mov: TAG = GEN diff --git a/tests/ref/fate/mov-stsd-v2-sample-rate-zero b/tests/ref/fate/mov-stsd-v2-sample-rate-zero new file mode 100644 index 0000000000..a39c5a26f4 --- /dev/null +++ b/tests/ref/fate/mov-stsd-v2-sample-rate-zero @@ -0,0 +1 @@ +stream|sample_rate=96000 -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
