Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PLI dilution for 0.x. Fixes-according-to-PR-3422-remark #3476

Open
wants to merge 3 commits into
base: 0.x
Choose a base branch
from
Open

PLI dilution for 0.x. Fixes-according-to-PR-3422-remark #3476

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
511bc44
PLI dilution for 0.x. Fixes-according-to-PR-3422-remark
natikaltura Nov 7, 2024
7e4367f
Fixed log format specifier
natikaltura Nov 11, 2024
3134f8e
Fixed according to PR remarks
natikaltura Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions plugins/janus_videoroom.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1703,6 +1703,7 @@ typedef struct janus_videoroom_publisher {
gint64 fir_latest; /* Time of latest sent FIR (to avoid flooding) */
gint fir_seq; /* FIR sequence number */
volatile gint need_pli; /* Whether we need to send a PLI later */
volatile gint sending_pli; /* Whether we're currently sending a PLI */
gboolean recording_active; /* Whether this publisher has to be recorded or not */
gchar *recording_base; /* Base name for the recording (e.g., /path/to/filename, will generate /path/to/filename-audio.mjr and/or /path/to/filename-video.mjr) */
janus_recorder *arc; /* The Janus recorder instance for this publisher's audio, if enabled */
Expand Down Expand Up @@ -1969,19 +1970,28 @@ static void janus_videoroom_codecstr(janus_videoroom *videoroom, char *audio_cod
static void janus_videoroom_reqpli(janus_videoroom_publisher *publisher, const char *reason) {
if(publisher == NULL || g_atomic_int_get(&publisher->destroyed))
return;
if(!g_atomic_int_compare_and_exchange(&publisher->sending_pli, 0, 1))
return;

gint64 now = janus_get_monotonic_time();
if(now - publisher->fir_latest < 100000) {
/* We just sent a PLI less than 100ms ago, schedule a new delivery later */
if(now - publisher->fir_latest < G_USEC_PER_SEC) {
/* We just sent a PLI less than a second ago, schedule a new delivery later */
g_atomic_int_set(&publisher->need_pli, 1);
g_atomic_int_set(&publisher->sending_pli, 0);
return;
}

/* Send a PLI */
JANUS_LOG(LOG_VERB, "%s sending PLI to %s (%s)\n", reason,
publisher->user_id_str, publisher->display ? publisher->display : "??");

g_atomic_int_set(&publisher->need_pli, 0);
publisher->fir_latest = janus_get_monotonic_time();

gateway->send_pli(publisher->session->handle);

/* Update the time of when we last sent a keyframe request */
publisher->fir_latest = now;
g_atomic_int_set(&publisher->need_pli, 0);
g_atomic_int_set(&publisher->sending_pli, 0);
}

/* Error codes */
Expand Down Expand Up @@ -8545,8 +8555,7 @@ static void janus_videoroom_relay_rtp_packet(gpointer data, gpointer user_data)
if(subscriber->sim_context.need_pli && subscriber->feed && subscriber->feed->session &&
subscriber->feed->session->handle) {
/* Send a PLI */
JANUS_LOG(LOG_VERB, "We need a PLI for the simulcast context\n");
gateway->send_pli(subscriber->feed->session->handle);
janus_videoroom_reqpli(subscriber->feed, "Simulcast context");
}
/* Do we need to drop this? */
if(!relay)
Expand Down
7 changes: 4 additions & 3 deletions rtp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,8 @@ gboolean janus_rtp_simulcasting_context_process_rtp(janus_rtp_simulcasting_conte
context->last_relayed = now;
} else if(context->substream > 0) {
/* Check if too much time went by with no packet relayed */
if((now - context->last_relayed) > (context->drop_trigger ? context->drop_trigger : 250000)) {
gint64 delay_us = (now - context->last_relayed);
if(delay_us > (context->drop_trigger ? context->drop_trigger : 250000)) {
context->last_relayed = now;
if(context->substream != substream && context->substream_target_temp != 0) {
if(context->substream_target > substream) {
Expand All @@ -1173,8 +1174,8 @@ gboolean janus_rtp_simulcasting_context_process_rtp(janus_rtp_simulcasting_conte
if(context->substream_target_temp < 0)
context->substream_target_temp = 0;
if(context->substream_target_temp != prev_target) {
JANUS_LOG(LOG_WARN, "No packet received on substream %d for a while, falling back to %d\n",
context->substream, context->substream_target_temp);
JANUS_LOG(LOG_WARN, "No packet received on substream %d for %d Milliseconds, falling back to %d\n",
Copy link
Contributor

@tmatth tmatth Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want PRId64 instead of %d here, although Janus currently isn't using this anywhere.

I would've suggested G_GINT64_MODIFIER but:

This is not necessarily the correct modifier for printing and scanning int64_t values, even though the in-memory representation is the same. Standard C macros like PRId64 and SCNd64 should be used for int64_t.

See:
https://docs.gtk.org/glib/types.html

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our code style is to use %"SCNi64" in those instances (assuming this is a int64_t?).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, that makes since given that internally janus_vprintf is calling g_vasprintf.

context->substream, (delay_us / 1000), context->substream_target_temp);
/* Notify the caller that we (still) need a PLI */
context->need_pli = TRUE;
}
Expand Down