Fix latency issue with rl pacing - #1649
Conversation
02a56af to
9b04ad5
Compare
9b04ad5 to
9831aaf
Compare
dcfa843 to
617c0b6
Compare
9831aaf to
6c20f67
Compare
257fe07 to
b67b50d
Compare
6c20f67 to
db1e27f
Compare
db1e27f to
dc4e56b
Compare
Hold the first content packet until its target TSC and derive default gates from the same media-clock instant as the RTP timestamp. Preserve exact-user TAI requests and cover ST20/ST40 timing boundaries. Signed-off-by: Kasiewicz, Marek <marek.kasiewicz@intel.com>
Release held mbufs on both ports before replacing shared pools while preserving sibling queue recovery requests. Cover repeated cleanup, simultaneous requests, and partial-burst ownership. Signed-off-by: Kasiewicz, Marek <marek.kasiewicz@intel.com>
Check every default, user, exact, jittered, and interlaced frame against a hardware-tested 300 us regression ceiling. Keep RTP timing and frame-count checks strict without percentage outlier state. Signed-off-by: Kasiewicz, Marek <marek.kasiewicz@intel.com>
Run build and test tools with bounded decoded output and checkout-local MTL and DPDK libraries. Keep gtest routing shell-free and report actionable NoCtx assertion failures. Signed-off-by: Kasiewicz, Marek <marek.kasiewicz@intel.com>
dc4e56b to
ce013ef
Compare
DawidWesierski4
left a comment
There was a problem hiding this comment.
Gonna do another PR for now this all
| @@ -919,6 +919,17 @@ static uint64_t st_muldiv_u64_round_closest(uint64_t value, uint64_t multiplier, | |||
| return (uint64_t)quotient; | |||
| } | |||
|
|
|||
| uint64_t st_tai_round_to_media_clk_ns(uint64_t tai_ns, uint32_t sampling_rate) { | |||
| if (!sampling_rate) { | |||
| err("%s, invalid sampling rate\n", __func__); | |||
| return 0; | |||
| } | |||
|
|
|||
| uint64_t tick = st_muldiv_u64_round_closest(tai_ns, sampling_rate, NS_PER_S); | |||
| if ((__uint128_t)tick * NS_PER_S / sampling_rate >= UINT64_MAX) return UINT64_MAX; | |||
| return st_muldiv_u64_round_closest(tick, NS_PER_S, sampling_rate); | |||
| } | |||
|
|
|||
| uint32_t st10_tai_to_media_clk(uint64_t tai_ns, uint32_t sampling_rate) { | |||
| if (!sampling_rate) { | |||
| err("%s, invalid sampling rate\n", __func__); | |||
There was a problem hiding this comment.
NITPIC (it already is a mess here):
if we are making those public then they don't belong in st_fmt.c how are those funcitons connected to format conversion? Those should go to utils or st_api
nothing to do with fortmat conversion
| if (!((s->ops.flags & ST20_TX_FLAG_EXACT_USER_PACING) && required_tai) && | ||
| !(s->ops.flags & | ||
| (ST20_TX_FLAG_USER_TIMESTAMP | ST20_TX_FLAG_RTP_TIMESTAMP_EPOCH)) && | ||
| !s->ops.rtp_timestamp_delta_us) { | ||
| start_time_tai = | ||
| st_tai_round_to_media_clk_ns(start_time_tai, s->fps_tm.sampling_clock_rate); |
There was a problem hiding this comment.
NITPIC(very corner corner case idk if you change it):
the required_tai could be 0, we are doing so in the tests
i think much more elegant approach would be to do somthing like this as required_tai being 0 and user pacing existing is an option
if (!(s->ops.flags & (ST20_TX_FLAG_EXACT_USER_PACING | ST20_TX_FLAG_USER_TIMESTAMP | ST20_TX_FLAG_RTP_TIMESTAMP_EPOCH)) && !s->ops.rtp_timestamp_delta_us) {
start_time_tai =
st_tai_round_to_media_clk_ns(start_time_tai, s->fps_tm.sampling_clock_rate);
}
| } | ||
|
|
||
| return 0; | ||
| return ret; |
There was a problem hiding this comment.
NITPICK: we should propablly check the result then ?
| if (s->inflight[i][0]) rte_pktmbuf_free_bulk(s->inflight[i], s->bulk); | ||
| if (s->trs_inflight_num[i]) | ||
| rte_pktmbuf_free_bulk(&s->trs_inflight[i][s->trs_inflight_idx[i]], | ||
| s->trs_inflight_num[i]); | ||
| if (s->trs_inflight_num2[i]) | ||
| rte_pktmbuf_free_bulk(&s->trs_inflight2[i][s->trs_inflight_idx2[i]], | ||
| s->trs_inflight_num2[i]); | ||
| for (unsigned int j = 0; j < s->trs_pad_inflight_num[i]; j++) { | ||
| if (s->pad[i][ST20_PKT_TYPE_NORMAL]) | ||
| rte_pktmbuf_free(s->pad[i][ST20_PKT_TYPE_NORMAL]); |
There was a problem hiding this comment.
There should be a mention about a registered rte free safty here ?
What if the registered cleanup flow per packet gonna go when this clenup works ?
Is this safe ?
| /* Payload pools are shared by redundant ports, so release all software-held | ||
| * mbufs before replacing them. */ | ||
| st_tx_video_transmitter_state_cleanup(s); | ||
| uint64_t recovery_tsc = mt_get_tsc(impl); | ||
| for (uint8_t i = 0; i < s->ops.num_port; i++) | ||
| s->last_burst_succ_time_tsc[i] = recovery_tsc; |
There was a problem hiding this comment.
there is no test covrege for this, we should probs add some as this is a problematic flow
No description provided.