From 4be56219cb2c0bfaa9c323b2a1e57be6bcd24d13 Mon Sep 17 00:00:00 2001 From: aviv ron Date: Mon, 10 Aug 2026 17:25:14 +0300 Subject: [PATCH] Bound audio duration before any transcription runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review finding #6 (P1, audio duration unbounded and ASR runs before the length check). asr_max_audio_clips bounds how many clips a request may carry but says nothing about their length, and vLLM's prompt-length check runs *after* preprocessing. So the documented "oversized requests get HTTP 400" meant a multi-hour file was transcribed in full and only then discarded: a free denial-of-service lever for an unauthenticated caller, and a synchronous block of vLLM's input path for as long as the transcription took. Three config fields, validated at construction: - asr_max_audio_seconds_per_clip (default 600.0) — longest single clip - asr_max_total_audio_seconds (default 1800.0) — longest total per request - asr_max_audio_samples (default 0 = derived from the total at 16 kHz) — an absolute decoded-sample cap as a rate-independent backstop Validation rejects NaN and inf as well as non-positive values. That is not pedantry: NaN compares False against every threshold, so an unvalidated one would silently *disable* the limit it configures rather than loosen it — the same hole finding #12 describes elsewhere in this config. Enforced in apply(), ahead of _call_hf_processor, so a rejected request loads no ASR model and transcribes, chunks and resamples nothing. Errors name the offending size and the knob that rejected it. The defaults are a policy choice, not a technical limit. They bound the worst case without breaking the documented long-audio chunking; a deployment serving interactive traffic will likely want them much tighter, and one serving long recordings will want them raised. Tests: 8 in the vLLM tier driving the real apply(), and 14 config-validation cases. Every rejection test installs a transcriber that raises if reached and then asserts it was never called, so the bar is "refused without touching ASR" rather than merely "raised" — the specific requirement in the finding. Negative controls cover within-limits, exactly-at-limit, defaults and text-only requests, so a guard that rejected everything would not pass. Verified as genuine by removing the call: 4 tests fail without it. The config tests live in tests/unit/test_config.py rather than beside the processor tests because ci.yaml runs only tests/unit/ on every PR. In the vLLM tier they would have been skipped wherever vLLM is absent, so the check that stops someone disabling this guard with a NaN would not have run on the PR introducing it. Two parts of the finding's required fix are deliberately not here, as both are separate work rather than additions to this change: an ASR timeout (interrupting a transformers pipeline call mid-flight) and bounded preprocessing concurrency (a semaphore whose scope interacts with vLLM's own input-path threading). The duration bounds close the unbounded-work hole; a timeout would close "bounded but still slow". Also note the audio has already been resampled to 16 kHz by vLLM's data parser by the time apply() sees it. Bounding that too would mean owning the parser. Transcription dominates the cost by a wide margin, so this closes the real lever, but "before decode, resample" is not fully satisfied. Signed-off-by: aviv ron --- docs/AUDIO.md | 17 +++ src/granite_switch/config.py | 34 ++++++ src/granite_switch/vllm/audio/processor.py | 86 ++++++++++++++ tests/unit/test_config.py | 45 ++++++++ tests/vllm/test_audio_processor.py | 124 +++++++++++++++++++++ 5 files changed, 306 insertions(+) diff --git a/docs/AUDIO.md b/docs/AUDIO.md index 8cf88bb..c42e050 100644 --- a/docs/AUDIO.md +++ b/docs/AUDIO.md @@ -106,6 +106,23 @@ Shorten the audio or serve with a larger `--max-model-len`. Relevant config fiel Clips cost no extra KV (transcripts are ordinary text tokens bounded by the context); the ceiling guards against one request triggering an unbounded number of synchronous transcriptions. +- `asr_max_audio_seconds_per_clip` (default `600.0`) — longest single clip. +- `asr_max_total_audio_seconds` (default `1800.0`) — longest total across all + clips in one request. +- `asr_max_audio_samples` (default `0` = derive from the total above at 16 kHz) — + absolute decoded-sample cap, as a rate-independent backstop. + +These three bound *duration*, which the clip count does not. They are enforced +**before any transcription runs**, which matters because vLLM's prompt-length +check happens after preprocessing: without them a caller could have a multi-hour +file fully transcribed and only then rejected — a free denial-of-service lever, +and a synchronous block of vLLM's input path for as long as the transcription +takes. An over-long request is refused with a message naming the offending size +and the knob that rejected it. + +Raise them if you serve genuinely long recordings; the defaults are a policy +choice, not a technical limit. Note that long *single* clips are still handled by +chunking (below) — these limits cap the input, not the transcript. **Long single clips** are handled two ways, selected by `asr_self_chunks`: diff --git a/src/granite_switch/config.py b/src/granite_switch/config.py index cc788d8..7e340c8 100644 --- a/src/granite_switch/config.py +++ b/src/granite_switch/config.py @@ -1,6 +1,8 @@ # SPDX-License-Identifier: Apache-2.0 """Configuration for Granite model with adapter switching.""" +import math + from transformers import GraniteMoeHybridConfig # Accepted asr_dtype values. Keep in sync with vllm.audio.asr._ASR_DTYPE_NAMES. @@ -63,6 +65,17 @@ class GraniteSwitchConfig(GraniteMoeHybridConfig): synchronous transcriptions one request can trigger and the startup profiling pass; ``--limit-mm-per-prompt`` may lower it, not raise it. Default: 32. + asr_max_audio_seconds_per_clip (float): Longest single clip accepted, + in seconds. Enforced before any transcription, so an oversized + clip is rejected instead of transcribed and then discarded. + Default: 600.0 (10 min). + asr_max_total_audio_seconds (float): Longest total audio accepted + across all clips in one request, in seconds. Default: 1800.0 + (30 min). + asr_max_audio_samples (int): Absolute cap on decoded samples per + request, as a rate-independent backstop to the second-based + limits. ``0`` derives it from asr_max_total_audio_seconds at the + 16 kHz working rate. Default: 0. asr_chunk_length_s (float): Chunker window length in seconds. Only used when asr_self_chunks is False. Default: 30.0. asr_chunk_overlap_s (float): Overlap in seconds between chunker @@ -98,6 +111,9 @@ def __init__( asr_pipeline_kwargs: dict | None = None, asr_generate_kwargs: dict | None = None, asr_max_audio_clips: int = 32, + asr_max_audio_seconds_per_clip: float = 600.0, + asr_max_total_audio_seconds: float = 1800.0, + asr_max_audio_samples: int = 0, asr_chunk_length_s: float = 30.0, asr_chunk_overlap_s: float = 5.0, asr_self_chunks: bool = True, @@ -203,7 +219,25 @@ def __init__( f"asr_chunk_overlap_s ({asr_chunk_overlap_s}) must be < " f"asr_chunk_length_s ({asr_chunk_length_s})" ) + # Duration bounds. Checked for finiteness as well as sign: NaN and inf + # compare False against every threshold, so an unvalidated one silently + # disables the limit it was meant to impose. + for name, value in ( + ("asr_max_audio_seconds_per_clip", asr_max_audio_seconds_per_clip), + ("asr_max_total_audio_seconds", asr_max_total_audio_seconds), + ): + if not math.isfinite(value) or value <= 0: + raise ValueError(f"{name} must be a finite number > 0, got {value!r}") + if not math.isfinite(asr_max_audio_samples) or asr_max_audio_samples < 0: + raise ValueError( + "asr_max_audio_samples must be a finite integer >= 0 " + f"(0 derives it from asr_max_total_audio_seconds), " + f"got {asr_max_audio_samples!r}" + ) self.asr_max_audio_clips = asr_max_audio_clips + self.asr_max_audio_seconds_per_clip = float(asr_max_audio_seconds_per_clip) + self.asr_max_total_audio_seconds = float(asr_max_total_audio_seconds) + self.asr_max_audio_samples = int(asr_max_audio_samples) self.asr_chunk_length_s = asr_chunk_length_s self.asr_chunk_overlap_s = asr_chunk_overlap_s self.asr_self_chunks = asr_self_chunks diff --git a/src/granite_switch/vllm/audio/processor.py b/src/granite_switch/vllm/audio/processor.py index ba6715e..e3426da 100644 --- a/src/granite_switch/vllm/audio/processor.py +++ b/src/granite_switch/vllm/audio/processor.py @@ -107,6 +107,24 @@ def _asr_max_audio_clips(self) -> int: cfg = self.get_hf_config() return int(getattr(cfg, "asr_max_audio_clips", 32) or 32) + def _asr_max_audio_seconds_per_clip(self) -> float: + cfg = self.get_hf_config() + value = getattr(cfg, "asr_max_audio_seconds_per_clip", None) + return float(value) if value else 600.0 + + def _asr_max_total_audio_seconds(self) -> float: + cfg = self.get_hf_config() + value = getattr(cfg, "asr_max_total_audio_seconds", None) + return float(value) if value else 1800.0 + + def _asr_max_audio_samples(self) -> int: + """Absolute decoded-sample cap; derived from the second-based total if 0.""" + cfg = self.get_hf_config() + value = getattr(cfg, "asr_max_audio_samples", None) + if value: + return int(value) + return int(self._asr_max_total_audio_seconds() * _TARGET_SR) + def _asr_self_chunks(self) -> bool: cfg = self.get_hf_config() return bool(getattr(cfg, "asr_self_chunks", True)) @@ -247,8 +265,76 @@ def apply(self, *args, **kwargs): """ prompt, num_audio_items = self._prompt_and_audio_count(*args, **kwargs) self._validate_marker_count(prompt, num_audio_items) + self._validate_audio_limits(*args, **kwargs) return super().apply(*args, **kwargs) + def _audio_items(self, *args, **kwargs): + """The request's parsed audio items, or ``None`` if unavailable.""" + inputs = args[0] if args else kwargs.get("inputs") + if hasattr(inputs, "mm_data_items"): + items = inputs.mm_data_items + else: + mm_data = args[1] if len(args) > 1 else kwargs.get("mm_data") + if not mm_data: + return None + items = self.info.get_data_parser().parse_mm_data(mm_data) + return items["audio"] if "audio" in items else None + + def _validate_audio_limits(self, *args, **kwargs) -> None: + """Reject over-long audio *before* anything transcribes it. + + ``asr_max_audio_clips`` bounds how many clips a request may carry but says + nothing about their length, and vLLM's prompt-length check runs *after* + preprocessing — so without this an unauthenticated caller can have a + multi-hour file fully transcribed and only then rejected. That is both a + free denial-of-service lever and a synchronous block of vLLM's input path + for the duration of the transcription. + + Enforced in ``apply()``, ahead of ``_call_hf_processor``, so no ASR model + is loaded and no audio is transcribed or chunked for a rejected request. + The audio has already been resampled to 16 kHz by the data parser at this + point, which is far cheaper than transcription but not free — bounding + that too would mean owning the parser. + """ + items = self._audio_items(*args, **kwargs) + if items is None or len(items) == 0: + return + + max_per_clip = self.info._asr_max_audio_seconds_per_clip() + max_total = self.info._asr_max_total_audio_seconds() + max_samples = self.info._asr_max_audio_samples() + + total_samples = 0 + for idx in range(len(items)): + try: + num_samples = items.get_audio_length(idx) + except (ValueError, AttributeError): + # A cached item carries no waveform to measure; it was bounded + # when it was first seen. + continue + total_samples += num_samples + seconds = num_samples / _TARGET_SR + if seconds > max_per_clip: + raise ValueError( + f"Audio clip {idx} is {seconds:.1f}s, over the " + f"{max_per_clip:.1f}s per-clip limit " + f"(asr_max_audio_seconds_per_clip). Split it or raise the " + f"limit; it is enforced before transcription runs." + ) + + total_seconds = total_samples / _TARGET_SR + if total_seconds > max_total: + raise ValueError( + f"Request carries {total_seconds:.1f}s of audio across " + f"{len(items)} clip(s), over the {max_total:.1f}s total limit " + f"(asr_max_total_audio_seconds)." + ) + if total_samples > max_samples: + raise ValueError( + f"Request decodes to {total_samples} audio samples, over the " + f"{max_samples} sample limit (asr_max_audio_samples)." + ) + def _transcribe( self, audio, diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 7ecbab4..2052c32 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -155,6 +155,51 @@ def test_overlap_ge_window_raises(self): num_adapters=0, asr_chunk_length_s=10.0, asr_chunk_overlap_s=10.0 ) + # ── Audio duration bounds ── + # These cap how much audio one request may carry, and are enforced before any + # transcription runs. NaN and inf are rejected explicitly: they compare False + # against every threshold, so an unvalidated one would silently disable the + # limit it configures rather than loosening it. + + def test_duration_limit_defaults(self): + cfg = GraniteSwitchConfig(num_adapters=0) + assert cfg.asr_max_audio_seconds_per_clip == 600.0 + assert cfg.asr_max_total_audio_seconds == 1800.0 + assert cfg.asr_max_audio_samples == 0 # 0 = derive from the total + + @pytest.mark.parametrize("bad", [0, -1.0, float("nan"), float("inf")]) + def test_invalid_seconds_per_clip_raises(self, bad): + with pytest.raises(ValueError, match="asr_max_audio_seconds_per_clip"): + GraniteSwitchConfig(num_adapters=0, asr_max_audio_seconds_per_clip=bad) + + @pytest.mark.parametrize("bad", [0, -1.0, float("nan"), float("inf")]) + def test_invalid_total_seconds_raises(self, bad): + with pytest.raises(ValueError, match="asr_max_total_audio_seconds"): + GraniteSwitchConfig(num_adapters=0, asr_max_total_audio_seconds=bad) + + @pytest.mark.parametrize("bad", [-1, float("nan"), float("inf")]) + def test_invalid_max_audio_samples_raises(self, bad): + with pytest.raises(ValueError, match="asr_max_audio_samples"): + GraniteSwitchConfig(num_adapters=0, asr_max_audio_samples=bad) + + def test_zero_max_audio_samples_allowed(self): + cfg = GraniteSwitchConfig(num_adapters=0, asr_max_audio_samples=0) + assert cfg.asr_max_audio_samples == 0 + + def test_duration_limits_round_trip(self, tmp_path): + GraniteSwitchConfig( + num_adapters=0, + asr_enabled=True, + asr_max_audio_seconds_per_clip=42.0, + asr_max_total_audio_seconds=84.0, + asr_max_audio_samples=1234, + ).save_pretrained(tmp_path) + loaded = GraniteSwitchConfig.from_pretrained(tmp_path) + + assert loaded.asr_max_audio_seconds_per_clip == 42.0 + assert loaded.asr_max_total_audio_seconds == 84.0 + assert loaded.asr_max_audio_samples == 1234 + def test_asr_kwargs_round_trip(self, tmp_path): # Pipeline/generate kwargs must survive save_pretrained → from_pretrained # so the checkpoint stays self-describing about its ASR front-end. diff --git a/tests/vllm/test_audio_processor.py b/tests/vllm/test_audio_processor.py index 367f6c1..220fe7e 100644 --- a/tests/vllm/test_audio_processor.py +++ b/tests/vllm/test_audio_processor.py @@ -811,3 +811,127 @@ def test_clean_transcript_is_unaffected(self, monkeypatch): info.get_tokenizer = lambda: _MarkerTokenizer() assert proc._transcribe(np.zeros(1600, dtype=np.float32)) == _TRANSCRIPT_IDS + + +class TestAudioDurationLimits: + """Over-long audio must be refused before anything transcribes it. + + ``asr_max_audio_clips`` bounds clip *count* only, and vLLM's prompt-length + check runs after preprocessing — so without a duration bound a caller can + have a multi-hour file fully transcribed and only then rejected. The bar here + is not just "raises" but "raises without the ASR pipeline being touched". + """ + + def _items(self, *durations_s, sr=16_000): + from vllm.multimodal.parse import AudioProcessorItems, MultiModalDataItems + + clips = [ + np.zeros(int(seconds * sr), dtype=np.float32) for seconds in durations_s + ] + return MultiModalDataItems({"audio": AudioProcessorItems(clips)}) + + def _proc(self, monkeypatch, **cfg): + """Processor whose transcriber records whether it was ever reached.""" + info = _make_info(asr_enabled=True, asr_model_id="w", **cfg) + info.get_tokenizer = lambda: _MarkerTokenizer() + proc = object.__new__(GraniteSwitchASRMultiModalProcessor) + proc.info = info + + calls = [] + + def fake_get_transcriber(**kw): + calls.append(kw) + raise AssertionError("ASR must not be reached for a rejected request") + + monkeypatch.setattr(proc_mod, "get_transcriber", fake_get_transcriber) + + from vllm.multimodal.processing import BaseMultiModalProcessor + + monkeypatch.setattr( + BaseMultiModalProcessor, + "apply", + lambda self, *a, **k: _DELEGATED, + raising=False, + ) + return proc, calls + + def _apply(self, proc, items): + prompt = proc_mod.AUDIO_MARKER * len(items["audio"]) + return proc.apply(SimpleNamespace(prompt=prompt, mm_data_items=items)) + + def test_overlong_clip_rejected_without_transcribing(self, monkeypatch): + proc, calls = self._proc(monkeypatch, asr_max_audio_seconds_per_clip=60.0) + + with pytest.raises(ValueError, match="per-clip limit"): + self._apply(proc, self._items(90.0)) + + assert calls == [], "ASR was invoked for a request that should be rejected" + + def test_total_across_clips_rejected_without_transcribing(self, monkeypatch): + proc, calls = self._proc( + monkeypatch, + asr_max_audio_seconds_per_clip=60.0, + asr_max_total_audio_seconds=100.0, + ) + + # Each clip is legal on its own; together they are not. + with pytest.raises(ValueError, match="total limit"): + self._apply(proc, self._items(50.0, 50.0, 50.0)) + + assert calls == [] + + def test_sample_cap_rejected_without_transcribing(self, monkeypatch): + """The rate-independent backstop fires even when the seconds pass.""" + proc, calls = self._proc( + monkeypatch, + asr_max_audio_seconds_per_clip=1000.0, + asr_max_total_audio_seconds=1000.0, + asr_max_audio_samples=16_000, # 1 second's worth + ) + + with pytest.raises(ValueError, match="sample limit"): + self._apply(proc, self._items(5.0)) + + assert calls == [] + + def test_error_names_the_offending_size_and_knob(self, monkeypatch): + proc, _ = self._proc(monkeypatch, asr_max_audio_seconds_per_clip=30.0) + + with pytest.raises(ValueError) as exc: + self._apply(proc, self._items(45.0)) + + message = str(exc.value) + assert "45.0s" in message and "30.0s" in message + assert "asr_max_audio_seconds_per_clip" in message + + def test_within_limits_is_accepted(self, monkeypatch): + """Negative control: legal audio must still get through.""" + proc, _ = self._proc( + monkeypatch, + asr_max_audio_seconds_per_clip=60.0, + asr_max_total_audio_seconds=120.0, + ) + + assert self._apply(proc, self._items(10.0, 20.0)) is _DELEGATED + + def test_boundary_is_inclusive(self, monkeypatch): + """Exactly at the limit is allowed; the check is > not >=.""" + proc, _ = self._proc(monkeypatch, asr_max_audio_seconds_per_clip=10.0) + + assert self._apply(proc, self._items(10.0)) is _DELEGATED + + def test_defaults_allow_ordinary_clips(self, monkeypatch): + """A checkpoint with no limits configured keeps working.""" + proc, _ = self._proc(monkeypatch) + + assert self._apply(proc, self._items(5.0)) is _DELEGATED + + def test_text_only_request_unaffected(self, monkeypatch): + from vllm.multimodal.parse import MultiModalDataItems + + proc, _ = self._proc(monkeypatch) + empty = MultiModalDataItems({}) + + assert proc.apply(SimpleNamespace(prompt="hi", mm_data_items=empty)) is ( + _DELEGATED + )