Handle redundancy payload type mapping ensuring symmetric PT

[trengginas]

PR #4344 added support for RT Text, including text stream redundancy level.
The default PT is configured from PJMEDIA_RTP_PT_REDUNDANCY/PJMEDIA_RTP_PT_T140.
When PJMEDIA_SDP_NEG_ANSWER_SYMMETRIC_PT is set, the library will rewrite the PT number to match the PT number as the remote offer.
Currently, the redundancy PT is not updated and still hardcoded to PJMEDIA_RTP_PT_T140 instead matching to the offer.
e,g: offer:

m=text 19742 RTP/SAVP 112 111
a=sendrecv
a=rtpmap:112 red/1000
a=fmtp:112 111/111/111
a=rtpmap:111 t140/1000

answer:

m=text 12026 RTP/SAVP 112 111
a=sendrecv
a=rtpmap:112 red/1000
a=fmtp:112 98/98/98
a=rtpmap:111 t140/1000

This PR will update the redundancy PT based on the offer to ensure the symmetric PT.

[Copilot]

Pull Request Overview

This PR fixes an issue with symmetric payload type (PT) handling for RT Text redundancy streams in SDP negotiation. When PJMEDIA_SDP_NEG_ANSWER_SYMMETRIC_PT is enabled, the library now properly updates redundancy PT numbers in fmtp attributes to match the remote offer, instead of keeping them hardcoded.

• Added text redundancy level configuration update in account modification
• Enhanced SDP negotiation to handle redundancy PT mapping symmetrically
• Updated fmtp attribute processing to rewrite redundancy PT values based on offer

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
pjsip/src/pjsua-lib/pjsua_acc.c	Adds text redundancy level configuration update during account modification
pjmedia/src/pjmedia/sdp_neg.c	Implements symmetric PT handling for redundancy streams by updating fmtp attributes

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

The magic number 32 for MAX_FMTP_STR_LEN should be defined as a named constant at the file or function level to improve maintainability and make the buffer size limit more explicit.

Suggested change

enum { MAX_FMTP_STR_LEN = 32 };

[trengginas]

Defining const as enum is common use in other parts of the code.

[Copilot]

The buffer length calculation using PJ_MIN could lead to truncated output without proper error handling. If the buffer becomes full, the function continues processing but may produce incomplete fmtp strings. Consider adding bounds checking and error handling.

Suggested change

	}
	if ((unsigned)len >= MAX_FMTP_STR_LEN - buf_len) {
	/* Truncation occurred, stop processing further. */
	buf_len = MAX_FMTP_STR_LEN - 1;
	buf[buf_len] = '\0';
	PJ_LOG(2, (THIS_FILE, "FMTP string truncated due to buffer size limit"));
	break;
	} else {
	buf_len += len;
	}

[Copilot]

The return value of pj_ansi_snprintf is not checked for potential buffer overflow. If the formatted string exceeds MAX_FMTP_STR_LEN, this could lead to undefined behavior. Add bounds checking.

Suggested change

	buf_len = pj_ansi_snprintf(buf, MAX_FMTP_STR_LEN, "%d ", pt_o);
	buf_len = pj_ansi_snprintf(buf, MAX_FMTP_STR_LEN, "%d ", pt_o);
	if (buf_len < 0 || buf_len >= MAX_FMTP_STR_LEN) {
	/* Truncate and null-terminate to be safe */
	buf_len = MAX_FMTP_STR_LEN - 1;
	buf[buf_len] = '\0';
	}

[trengginas]

With MAX_FMTP_STR_LEN is set to 32, buffer overflow most likely not going to happen

[sauwming]

If I'm understanding this correctly, it looks like we're searching for the text pt for every redundancy level (instead of just once)?

[trengginas]

That is correct. The goal is to be generic, since redundant audio data might have a different number.
e.g.: https://datatracker.ietf.org/doc/html/rfc2198#section-5

       m=audio 12345 RTP/AVP 121 0 5
       a=rtpmap:121 red/8000/1
       a=fmtp:121 0/5