[1.x] Suggestion: (videoroom.c) keep publisher's stream info consistent in notifying others and returning to self

[jamken]

What version of Janus is this happening on?
v1.0.2

Have you tested a more recent version of Janus too?
Yes, the latest one

The code in janus_videoroom_notify_about_publisher():

		json_object_set_new(info, "type", json_string(janus_videoroom_media_str(ps->type)));
		json_object_set_new(info, "mindex", json_integer(ps->mindex));
		json_object_set_new(info, "mid", json_string(ps->mid));
		if(ps->disabled) {
			json_object_set_new(info, "disabled", json_true());
		} else {
			if(ps->description)
				json_object_set_new(info, "description", json_string(ps->description));
			if(ps->type == JANUS_VIDEOROOM_MEDIA_AUDIO) {
				json_object_set_new(info, "codec", json_string(janus_audiocodec_name(ps->acodec)));
				/* FIXME For backwards compatibility, we need audio_codec in the global info */
				if(!audio_added) {
					audio_added = TRUE;
					json_object_set_new(pl, "audio_codec", json_string(janus_audiocodec_name(ps->acodec)));
				}
				if(ps->acodec == JANUS_AUDIOCODEC_OPUS) {
					if(ps->opusstereo)
						json_object_set_new(info, "stereo", json_true());
					if(ps->opusfec)
						json_object_set_new(info, "fec", json_true());
					if(ps->opusdtx)
						json_object_set_new(info, "dtx", json_true());
				}
			} else if(ps->type == JANUS_VIDEOROOM_MEDIA_VIDEO) {
				json_object_set_new(info, "codec", json_string(janus_videocodec_name(ps->vcodec)));
				/* FIXME For backwards compatibility, we need video_codec in the global info */
				if(!video_added) {
					video_added = TRUE;
					json_object_set_new(pl, "video_codec", json_string(janus_videocodec_name(ps->vcodec)));
				}
				if(ps->vcodec == JANUS_VIDEOCODEC_H264 && ps->h264_profile != NULL)
					json_object_set_new(info, "h264_profile", json_string(ps->h264_profile));
				else if(ps->vcodec == JANUS_VIDEOCODEC_VP9)
					json_object_set_new(info, "vp9_profile", json_string(ps->vp9_profile));
				if(ps->muted)
					json_object_set_new(info, "moderated", json_true());
				if(ps->simulcast)
					json_object_set_new(info, "simulcast", json_true());
				if(ps->svc)
					json_object_set_new(info, "svc", json_true());
			}
		}
		json_array_append_new(media, info);
		temp = temp->next;
	}

And the code from line 8047 to line 8099:

						json_object_set_new(info, "type", json_string(janus_videoroom_media_str(ps->type)));
						json_object_set_new(info, "mindex", json_integer(ps->mindex));
						json_object_set_new(info, "mid", json_string(ps->mid));

						if(ps->disabled) {
							json_object_set_new(info, "disabled", json_true());
						} else {
							if(ps->description)
								json_object_set_new(info, "description", json_string(ps->description));
							if(ps->type == JANUS_VIDEOROOM_MEDIA_AUDIO) {
								json_object_set_new(info, "codec", json_string(janus_audiocodec_name(ps->acodec)));
								/* FIXME For backwards compatibility, we need audio_codec in the global info */
								if(!audio_added) {
									audio_added = TRUE;
									json_object_set_new(pl, "audio_codec", json_string(janus_audiocodec_name(ps->acodec)));
								}
								if(ps->acodec == JANUS_AUDIOCODEC_OPUS) {
									if(ps->opusstereo)
										json_object_set_new(info, "stereo", json_true());
									if(ps->opusfec)
										json_object_set_new(info, "fec", json_true());
									if(ps->opusdtx)
										json_object_set_new(info, "dtx", json_true());
								}
								if(ps->audio_level_extmap_id > 0) {
									json_object_set_new(info, "talking", talking ? json_true() : json_false());
									/* FIXME For backwards compatibility, we also need talking in the global info */
									talking_found = TRUE;
									talking |= ps->talking;
								}
							} else if(ps->type == JANUS_VIDEOROOM_MEDIA_VIDEO) {
								/* FIXME For backwards compatibility, we need video_codec in the global info */
								json_object_set_new(info, "codec", json_string(janus_videocodec_name(ps->vcodec)));
								if(!video_added) {
									video_added = TRUE;
									json_object_set_new(pl, "video_codec", json_string(janus_videocodec_name(ps->vcodec)));
								}
								if(ps->vcodec == JANUS_VIDEOCODEC_H264 && ps->h264_profile != NULL)
									json_object_set_new(info, "h264_profile", json_string(ps->h264_profile));
								else if(ps->vcodec == JANUS_VIDEOCODEC_VP9 && ps->vp9_profile != NULL)
									json_object_set_new(info, "vp9_profile", json_string(ps->vp9_profile));
								if(ps->simulcast)
									json_object_set_new(info, "simulcast", json_true());
								if(ps->svc)
									json_object_set_new(info, "svc", json_true());
							}
							if(ps->muted)
								json_object_set_new(info, "moderated", json_true());
						}
						json_array_append_new(media, info);
						temp = temp->next;
					}
					json_object_set_new(pl, "streams", media);

The above two pieces of code is used to tell the publisher's stream info to others.

The follwoing code from line 10709 to line 10733 is used to return the publisher's stream info to self, but it does not contains the same field with the above code:

					/* Add to the info we send back to the publisher */
					json_t *info = json_object();
					json_object_set_new(info, "type", json_string(janus_videoroom_media_str(ps->type)));
					json_object_set_new(info, "mindex", json_integer(ps->mindex));
					json_object_set_new(info, "mid", json_string(ps->mid));
					if(ps->disabled) {
						json_object_set_new(info, "disabled", json_true());
					} else {
						if(ps->description)
							json_object_set_new(info, "description", json_string(ps->description));
						if(ps->type == JANUS_VIDEOROOM_MEDIA_AUDIO) {
							json_object_set_new(info, "codec", json_string(janus_audiocodec_name(ps->acodec)));
							if(ps->opusfec)
								json_object_set_new(info, "opus-fec", json_true());
						} else if(ps->type == JANUS_VIDEOROOM_MEDIA_VIDEO) {
							json_object_set_new(info, "codec", json_string(janus_videocodec_name(ps->vcodec)));
							if(ps->simulcast)
								json_object_set_new(info, "simulcast", json_true());
							if(ps->svc)
								json_object_set_new(info, "svc", json_true());
						}
					}
					json_array_append_new(media, info);
				}
				janus_mutex_unlock(&participant->streams_mutex);

I think it makes more sence to keep the above case with the same info, which is more convinient for client to get self's negotiated stream info without parsing jsep answer

[lminiero]

I guess it depends on whether what he have now for publishers is simply a subset of what we send other people, or if there are different properties. If they're different, we may be causing a breaking change, as there may be people relying on what we send right now.

[jamken]

I think It may be not a breaking change if just supplimenting some new properties to return self as to notify others.

[lminiero]

which is more convinient for client to get self's negotiated stream info without parsing jsep answer

I think the vast majority of info others get, you get already: the negotiated codec is definitelty there for instance. I only see a few info on Opus missing (stereo/dtx) and the H.264/VP9 profile: I don't think either is fundamentally important, but I'll add them just for the same of completeness.