Attention is currently required from: pespin, dexter.

Patch set 4:Code-Review -1

View Change

5 comments:

• Patchset:

  • Patch Set #4:

    CR-1 due to TS 101.138 vs TS 101.318.

• File include/osmo-bts/bts.h:

  • Patch Set #4, Line 71: 101.138

    TS 101.318

• File src/common/bts.c:

  • Patch Set #4, Line 208: 101.138

    TS 101.318

• File src/common/l1sap.c:

  • Patch Set #4, Line 1955: /* Since we already verified the payload in rtppayload_validate_hr(), we may trust that the payload length is

    I meant something like this (assuming no weird lengths like len=0 are to be received here): […]

    Agreeing with pespin here, his proposed approach looks more readable. Also, given that it's just one optional byte in front of the payload, the code can be simplified further:

    ```
    msg = l1sap_msgb_alloc(rtp_pl_len);
    if (!msg)
    	return NULL;
    memcpy(msgb_put(msg, rtp_pl_len), rtp_pl, rtp_pl_len);
    	
    switch (rtp_pl_len) {
    case GSM_HR_BYTES_RTP_TS101318:
    	/* Convert from TS 101 318 to RFC 5993 */
    	if (OSMO_UNLIKELY(!ts101318 && rfc5993))
    		msgb_push_u8(msg, 0x00);
    	break; 
    case GSM_HR_BYTES_RTP_RFC5993:
    	/* Convert from RFC 5993 TS 101 318 */
    	if (OSMO_UNLIKELY(!rfc5993 && ts101318))
    		msgb_pull_u8(msg);
    	break;
    default:
    	OSMO_ASSERT(0);
    }
    ```

• File src/osmo-bts-virtual/main.c:

  • Patch Set #4, Line 76: BTS_INTERNAL_FLAG_SPEECH_H_V1_RTP_RFC5993

    I am not 100% sure, but osmo-bts-virtual should in theory support both formats.

To view, visit change 32630. To unsubscribe, or for help writing mail filters, visit settings.