Skip to content

Commit e42b866

Browse files
committed
fix(quality): address Sonar findings
Signed-off-by: burnrazor <128198300+Dragon-kanji@users.noreply.github.com>
1 parent fad5d89 commit e42b866

9 files changed

Lines changed: 452 additions & 204 deletions

src/adaptive_bitrate.cpp

Lines changed: 222 additions & 135 deletions
Large diffs are not rendered by default.

src/adaptive_bitrate.h

Lines changed: 148 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,153 @@ namespace stream::adaptive_bitrate {
190190
unrecovered_loss,
191191
};
192192

193+
/**
194+
* @brief Timing and direction retained across encoder decisions.
195+
*/
196+
struct decision_history_t {
197+
decision_direction_e direction = decision_direction_e::none; ///< Latest non-probe command direction.
198+
std::optional<time_point_t> last_decision_at; ///< Time of the last emitted encoder command.
199+
std::optional<time_point_t> last_increase_at; ///< Time of the last recovery increase.
200+
std::optional<time_point_t> last_direction_reversal_at; ///< Latest non-probe command direction reversal.
201+
};
202+
203+
/**
204+
* @brief Handle malformed or protocol-mismatched telemetry.
205+
*
206+
* @param snapshot Telemetry snapshot to inspect.
207+
* @return `true` when the snapshot was handled and observation must stop.
208+
*/
209+
bool handle_invalid_telemetry(const network_metrics::snapshot_t &snapshot);
210+
211+
/**
212+
* @brief Hold adaptation after telemetry report limiting.
213+
*
214+
* @param snapshot Telemetry snapshot to inspect.
215+
* @return `true` when rate limiting was observed and observation must stop.
216+
*/
217+
bool handle_rate_limited_telemetry(const network_metrics::snapshot_t &snapshot);
218+
219+
/**
220+
* @brief Record whether a snapshot carries actionable loss telemetry.
221+
*
222+
* @param snapshot Telemetry snapshot to inspect.
223+
* @return `true` when the snapshot can update controller evidence.
224+
*/
225+
bool accept_telemetry_signal(const network_metrics::snapshot_t &snapshot);
226+
227+
/**
228+
* @brief Classify the strongest degradation present in one telemetry window.
229+
*
230+
* @param snapshot Telemetry snapshot to classify.
231+
* @param unrecovered_ratio Ratio of unrecovered data shards.
232+
* @param recovered_ratio Ratio of FEC-recovered data shards.
233+
* @param high_latency Whether latency corroborates recovery pressure.
234+
* @return Classified degradation, or `none` for a healthy window.
235+
*/
236+
static degradation_e classify_degradation(
237+
const network_metrics::snapshot_t &snapshot,
238+
long double unrecovered_ratio,
239+
long double recovered_ratio,
240+
bool high_latency
241+
);
242+
243+
/**
244+
* @brief Accumulate one degraded telemetry window.
245+
*
246+
* @param degradation Degradation observed in the window.
247+
* @param now Monotonic observation time.
248+
*/
249+
void observe_degradation(degradation_e degradation, time_point_t now);
250+
251+
/**
252+
* @brief Update recovery evidence from one non-degraded telemetry window.
253+
*
254+
* @param snapshot Telemetry snapshot to observe.
255+
* @param recovered_ratio Ratio of FEC-recovered data shards.
256+
* @param high_latency Whether latency remains elevated.
257+
* @param now Monotonic observation time.
258+
*/
259+
void observe_stability(
260+
const network_metrics::snapshot_t &snapshot,
261+
long double recovered_ratio,
262+
bool high_latency,
263+
time_point_t now
264+
);
265+
266+
/**
267+
* @brief Clear unconfirmed degraded-window evidence.
268+
*/
269+
void reset_degradation_confirmation();
270+
271+
/**
272+
* @brief Consume a fresh control-channel liveness sample.
273+
*
274+
* @param control_liveness_token Latest control-channel receive token.
275+
* @return `true` when the token is valid and was not previously consumed.
276+
*/
277+
bool consume_control_liveness(std::uint32_t control_liveness_token);
278+
279+
/**
280+
* @brief Update the stable RTT floor from live control-channel telemetry.
281+
*
282+
* @param now Monotonic control-loop time.
283+
* @param live_rtt_ms Current smoothed RTT.
284+
* @param live_rtt_variance_ms Current RTT variance.
285+
* @return `true` when live latency is materially elevated.
286+
*/
287+
bool observe_live_latency(time_point_t now, std::uint32_t live_rtt_ms, std::uint32_t live_rtt_variance_ms);
288+
289+
/**
290+
* @brief Check whether the encoder command spacing interval has elapsed.
291+
*
292+
* @param now Monotonic control-loop time.
293+
* @return `true` when another decision may be emitted.
294+
*/
295+
bool decision_is_allowed(time_point_t now) const;
296+
297+
/**
298+
* @brief Record and return one encoder decision.
299+
*
300+
* @param target_kbps Requested encoder target.
301+
* @param reason Stable diagnostic reason for the command.
302+
* @param now Monotonic decision time.
303+
* @return Recorded encoder decision.
304+
*/
305+
decision_t emit_decision(std::uint32_t target_kbps, decision_reason_e reason, time_point_t now);
306+
307+
/**
308+
* @brief Emit a decrease for confirmed degradation when possible.
309+
*
310+
* @param now Monotonic control-loop time.
311+
* @param decision_allowed Whether command spacing permits a decision.
312+
* @return A decrease command, or no value.
313+
*/
314+
std::optional<decision_t> reduce_for_pending_degradation(time_point_t now, bool decision_allowed);
315+
316+
/**
317+
* @brief Check all stable recovery prerequisites.
318+
*
319+
* @param now Monotonic control-loop time.
320+
* @param live_latency_high Whether live latency remains elevated.
321+
* @param fresh_control_liveness Whether a new reliable acknowledgement was observed.
322+
* @param decision_allowed Whether command spacing permits a decision.
323+
* @return `true` when recovery may increase the bitrate.
324+
*/
325+
bool recovery_is_allowed(
326+
time_point_t now,
327+
bool live_latency_high,
328+
bool fresh_control_liveness,
329+
bool decision_allowed
330+
) const;
331+
332+
/**
333+
* @brief Check whether an upward direction reversal remains in cooldown.
334+
*
335+
* @param now Monotonic control-loop time.
336+
* @return `true` while another upward reversal is blocked.
337+
*/
338+
bool upward_reversal_is_cooling_down(time_point_t now) const;
339+
193340
state_e state_ = state_e::fixed_disabled; ///< Current lifecycle state.
194341
std::uint32_t floor_kbps_ = 0; ///< Session floor, capped by the client ceiling.
195342
std::uint32_t ceiling_kbps_ = 0; ///< Immutable effective client ceiling.
@@ -205,11 +352,8 @@ namespace stream::adaptive_bitrate {
205352
bool telemetry_seen_ = false; ///< Whether a valid FEC report has been observed.
206353
bool fallback_after_acknowledgement_ = false; ///< Defer invalid-telemetry fallback while an encoder command is in flight.
207354
bool recovery_blocked_by_rate_limit_ = false; ///< Require a valid stable FEC report after telemetry overload.
208-
decision_direction_e last_decision_direction_ = decision_direction_e::none; ///< Latest non-probe command direction.
209355
std::optional<time_point_t> first_bad_window_at_; ///< Time of the first consecutive bad window.
210356
std::optional<time_point_t> stable_since_; ///< Start of the current no-new-pressure period.
211-
std::optional<time_point_t> last_decision_at_; ///< Time of the last emitted encoder command.
212-
std::optional<time_point_t> last_increase_at_; ///< Time of the last recovery increase.
213-
std::optional<time_point_t> last_direction_reversal_at_; ///< Latest non-probe command direction reversal.
357+
decision_history_t decision_history_; ///< Timing and direction of prior encoder decisions.
214358
};
215359
} // namespace stream::adaptive_bitrate

src/network_metrics.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,9 @@ namespace stream::network_metrics {
153153
const auto missing_data_packets = status->total_data_packets > status->received_data_packets ?
154154
static_cast<std::uint64_t>(status->total_data_packets - status->received_data_packets) :
155155
0;
156-
const auto received_shards = static_cast<std::uint32_t>(status->received_data_packets) + status->received_parity_packets;
157-
if (received_shards >= status->total_data_packets) {
156+
if (const auto received_shards =
157+
static_cast<std::uint32_t>(status->received_data_packets) + status->received_parity_packets;
158+
received_shards >= status->total_data_packets) {
158159
accumulator_.fec_recovered_data_packets += missing_data_packets;
159160
} else {
160161
accumulator_.unrecovered_data_packets += missing_data_packets;

src/nvenc/nvenc_reconfigure.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,15 @@ namespace NVENC_NAMESPACE {
8080
next_config.rcParams.maxBitRate = target_bps;
8181
if (baseline_vbv_buffer_size_ != 0) {
8282
auto scaled_vbv = scaled_from_baseline(baseline_vbv_buffer_size_);
83-
if (!scaled_vbv) {
83+
if (!scaled_vbv.has_value()) {
8484
result.status = video::bitrate_reconfigure_status_e::invalid;
8585
return result;
8686
}
8787
next_config.rcParams.vbvBufferSize = *scaled_vbv;
8888
}
8989
if (baseline_vbv_initial_delay_ != 0) {
9090
auto scaled_delay = scaled_from_baseline(baseline_vbv_initial_delay_);
91-
if (!scaled_delay) {
91+
if (!scaled_delay.has_value()) {
9292
result.status = video::bitrate_reconfigure_status_e::invalid;
9393
return result;
9494
}

src/stream.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,16 @@ namespace stream {
116116
* @return Stable lower-case diagnostic label.
117117
*/
118118
std::string_view adaptive_bitrate_reason_name(const adaptive_bitrate::decision_reason_e reason) {
119+
using enum adaptive_bitrate::decision_reason_e;
120+
119121
switch (reason) {
120-
case adaptive_bitrate::decision_reason_e::capability_check:
122+
case capability_check:
121123
return "capability_check"sv;
122-
case adaptive_bitrate::decision_reason_e::unrecovered_loss:
124+
case unrecovered_loss:
123125
return "unrecovered_loss"sv;
124-
case adaptive_bitrate::decision_reason_e::fec_pressure:
126+
case fec_pressure:
125127
return "fec_pressure"sv;
126-
case adaptive_bitrate::decision_reason_e::stable_recovery:
128+
case stable_recovery:
127129
return "stable_recovery"sv;
128130
}
129131

src/video.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,18 @@ using namespace std::literals;
4747
namespace video {
4848

4949
std::string_view bitrate_reconfigure_status_name(bitrate_reconfigure_status_e status) {
50+
using enum bitrate_reconfigure_status_e;
51+
5052
switch (status) {
51-
case bitrate_reconfigure_status_e::applied:
53+
case applied:
5254
return "applied";
53-
case bitrate_reconfigure_status_e::unchanged:
55+
case unchanged:
5456
return "unchanged";
55-
case bitrate_reconfigure_status_e::invalid:
57+
case invalid:
5658
return "invalid";
57-
case bitrate_reconfigure_status_e::unsupported:
59+
case unsupported:
5860
return "unsupported";
59-
case bitrate_reconfigure_status_e::failed:
61+
case failed:
6062
return "failed";
6163
}
6264

tests/unit/test_network_metrics.cpp

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,19 @@ namespace {
1919
using namespace std::chrono_literals;
2020
using stream::network_metrics::frame_fec_status_payload_size;
2121

22+
/** Fixed-size control payload used by the parser tests. */
23+
using frame_fec_status_payload_t = std::array<char, frame_fec_status_payload_size>;
24+
2225
/**
2326
* @brief Write a big-endian 16-bit integer into a test payload.
2427
*
2528
* @param payload Destination payload.
2629
* @param offset Byte offset to write.
2730
* @param value Host-endian value.
2831
*/
29-
void put_u16_be(std::array<std::uint8_t, frame_fec_status_payload_size> &payload, const std::size_t offset, const std::uint16_t value) {
30-
payload[offset] = static_cast<std::uint8_t>(value >> 8);
31-
payload[offset + 1] = static_cast<std::uint8_t>(value);
32+
void put_u16_be(frame_fec_status_payload_t &payload, const std::size_t offset, const std::uint16_t value) {
33+
payload[offset] = static_cast<char>(value >> 8);
34+
payload[offset + 1] = static_cast<char>(value);
3235
}
3336

3437
/**
@@ -38,11 +41,11 @@ namespace {
3841
* @param offset Byte offset to write.
3942
* @param value Host-endian value.
4043
*/
41-
void put_u32_be(std::array<std::uint8_t, frame_fec_status_payload_size> &payload, const std::size_t offset, const std::uint32_t value) {
42-
payload[offset] = static_cast<std::uint8_t>(value >> 24);
43-
payload[offset + 1] = static_cast<std::uint8_t>(value >> 16);
44-
payload[offset + 2] = static_cast<std::uint8_t>(value >> 8);
45-
payload[offset + 3] = static_cast<std::uint8_t>(value);
44+
void put_u32_be(frame_fec_status_payload_t &payload, const std::size_t offset, const std::uint32_t value) {
45+
payload[offset] = static_cast<char>(value >> 24);
46+
payload[offset + 1] = static_cast<char>(value >> 16);
47+
payload[offset + 2] = static_cast<char>(value >> 8);
48+
payload[offset + 3] = static_cast<char>(value);
4649
}
4750

4851
/**
@@ -56,15 +59,15 @@ namespace {
5659
* @param fec_percentage Sender FEC percentage encoded in the frame header.
5760
* @return Big-endian SS_FRAME_FEC_STATUS payload.
5861
*/
59-
std::array<std::uint8_t, frame_fec_status_payload_size> make_payload(
62+
frame_fec_status_payload_t make_payload(
6063
const std::uint16_t total_data,
6164
const std::uint16_t total_parity,
6265
const std::uint16_t received_data,
6366
const std::uint16_t received_parity,
6467
const std::uint16_t missing,
6568
const std::uint8_t fec_percentage = 30
6669
) {
67-
std::array<std::uint8_t, frame_fec_status_payload_size> payload {};
70+
frame_fec_status_payload_t payload {};
6871
put_u32_be(payload, 0, 42);
6972
put_u16_be(payload, 4, 120);
7073
put_u16_be(payload, 6, 115);
@@ -73,7 +76,7 @@ namespace {
7376
put_u16_be(payload, 12, total_parity);
7477
put_u16_be(payload, 14, received_data);
7578
put_u16_be(payload, 16, received_parity);
76-
payload[18] = fec_percentage;
79+
payload[18] = static_cast<char>(fec_percentage);
7780
payload[19] = 0;
7881
payload[20] = 1;
7982
return payload;
@@ -86,8 +89,8 @@ namespace {
8689
* @return String view spanning the payload bytes.
8790
*/
8891
template<std::size_t Size>
89-
std::string_view payload_view(const std::array<std::uint8_t, Size> &payload) {
90-
return {reinterpret_cast<const char *>(payload.data()), payload.size()};
92+
std::string_view payload_view(const std::array<char, Size> &payload) {
93+
return {payload.data(), payload.size()};
9194
}
9295
} // namespace
9396

@@ -116,8 +119,8 @@ TEST(FrameFecStatusTests, ParsesCurrentBigEndianWireFormat) {
116119
}
117120

118121
TEST(FrameFecStatusTests, RejectsPayloadWithWrongVersionSize) {
119-
std::array<std::uint8_t, frame_fec_status_payload_size - 1> short_payload {};
120-
std::array<std::uint8_t, frame_fec_status_payload_size + 1> long_payload {};
122+
std::array<char, frame_fec_status_payload_size - 1> short_payload {};
123+
std::array<char, frame_fec_status_payload_size + 1> long_payload {};
121124

122125
EXPECT_FALSE(stream::network_metrics::parse_frame_fec_status(payload_view(short_payload)));
123126
EXPECT_FALSE(stream::network_metrics::parse_frame_fec_status(payload_view(long_payload)));
@@ -198,7 +201,7 @@ TEST(NetworkMetricsTrackerTests, RequiresAdvertisedFeature) {
198201
TEST(NetworkMetricsTrackerTests, CountsMalformedPayloads) {
199202
const auto start = stream::network_metrics::time_point_t {};
200203
stream::network_metrics::tracker_t tracker {start};
201-
std::array<std::uint8_t, frame_fec_status_payload_size - 1> payload {};
204+
std::array<char, frame_fec_status_payload_size - 1> payload {};
202205

203206
EXPECT_EQ(
204207
tracker.ingest(payload_view(payload), true),

0 commit comments

Comments
 (0)