MediaPublish: use min_segment_wallclock_s for audio-only segment_time#22
MediaPublish: use min_segment_wallclock_s for audio-only segment_time#22chrishobcroft wants to merge 1 commit into
Conversation
The audio-only branch hardcoded segment_time to 2.0 s, which both set an unnecessary ~2 s latency floor on audio-only trickle publishers and made MediaPublishConfig.min_segment_wallclock_s look like a knob that does nothing on this path (it's only checked as a lower bound at the segment post-loop level; the PyAV segment muxer cuts well before it's consulted). For audio-only streams there's no keyframe constraint, so derive segment_time from min_segment_wallclock_s instead, with a 0.1 s floor to guard against pathological inputs. Default behaviour is unchanged for anyone who didn't set the field (1.0 s) — still shorter than the old 2.0 s hardcode. Video path is untouched. Measured on a mic-to-trickle audio-only publisher with min_segment_wallclock_s=0.3: before: POST cadence 1.5-1.9 s, segments 30-43 KB after: POST cadence 0.3-0.6 s, segments 6-13 KB Refs livepeer#21.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
I'll check this a little more, but in general 100ms is too small for a segment. You don't need small segments to get low latency. This statement might contribute to the confusion:
is not true; the segment is two seconds but this doesn't say anything about the latency, since data within a segment is transmitted as soon as it's available. If there is latency it is coming from somewhere else. |
The audio-only branch hardcoded segment_time to 2.0 s, which both set an unnecessary ~2 s latency floor on audio-only trickle publishers and made MediaPublishConfig.min_segment_wallclock_s look like a knob that does nothing on this path (it's only checked as a lower bound at the segment post-loop level; the PyAV segment muxer cuts well before it's consulted).
For audio-only streams there's no keyframe constraint, so derive segment_time from min_segment_wallclock_s instead, with a 0.1 s floor to guard against pathological inputs. Default behaviour is unchanged for anyone who didn't set the field (1.0 s) — still shorter than the old 2.0 s hardcode. Video path is untouched.
Measured on a mic-to-trickle audio-only publisher with min_segment_wallclock_s=0.3:
before: POST cadence 1.5-1.9 s, segments 30-43 KB
after: POST cadence 0.3-0.6 s, segments 6-13 KB
Fixes #21.