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.

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

Support larger values for SDP attributes #3282

Merged
merged 9 commits into from
Nov 20, 2023
4 changes: 2 additions & 2 deletions src/sdp-utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ janus_sdp_attribute *janus_sdp_attribute_create(const char *name, const char *va
a->direction = JANUS_SDP_DEFAULT;
a->value = NULL;
if(value) {
char buffer[512];
char buffer[2048];
va_list ap;
va_start(ap, value);
g_vsnprintf(buffer, sizeof(buffer), value, ap);
Expand Down Expand Up @@ -1119,7 +1119,7 @@ char *janus_sdp_write(janus_sdp *imported) {
if(!imported)
return NULL;
janus_refcount_increase(&imported->ref);
char *sdp = g_malloc(1024), mline[8192], buffer[512];
char *sdp = g_malloc(1024), mline[8192], buffer[2048];
Copy link
Member

Choose a reason for hiding this comment

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

If the buffer is this large, now, then I guess the sdp allocation we start from should be larger too? We only reallocate it when appending m-lines, which means it should be large enough to handle global attributes before that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, should then the sdplen variable match the g_malloc(size)?
So I should change to *char sdp = g_malloc(4096) and size_t sdplen = 4096?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lminiero - Committed an increase to 4096 of initial g_malloc and size_t sdplen

Copy link
Member

Choose a reason for hiding this comment

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

Mh I think it might be overkill, the vast majority of SDPs are never that large: I think we started from a lower value on purpose. I'll give this some thought and get back to you later on.

Copy link
Member

Choose a reason for hiding this comment

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

To be more conservative, maybe let's keep the same diff that was before, so 2048+512=2560? It's still large, but not THAT large.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, reduced to 2560 sdplen and g_malloc(size)

*sdp = '\0';
size_t sdplen = 1024, mlen = sizeof(mline), offset = 0, moffset = 0;
/* v= */
Expand Down
2 changes: 1 addition & 1 deletion src/sdp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1485,7 +1485,7 @@ char *janus_sdp_merge(void *ice_handle, janus_sdp *anon, gboolean offer) {
g_free(anon->c_addr);
anon->c_addr = NULL;
/* bundle: add new global attribute */
char buffer[8192], buffer_part[512];
char buffer[8192], buffer_part[2048];
buffer[0] = '\0';
buffer_part[0] = '\0';
g_snprintf(buffer, sizeof(buffer), "BUNDLE");
Expand Down