laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32088 )
Change subject: bts-{sysmo,oc2g,lc15}: Fix RTP of AMR SID_FIRST_P1
......................................................................
Patch Set 4: Verified+1 Code-Review+2
(1 comment)
Patchset:
PS4:
verified in a pcap file, it now does what it's supposed to do.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32088
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I90479efcc002d497648a71e73847af54e6208358
Gerrit-Change-Number: 32088
Gerrit-PatchSet: 4
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Mar 2023 15:23:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: pespin.
matanp has posted comments on this change. ( https://gerrit.osmocom.org/c/gapk/+/32094 )
Change subject: fmt_rtp_amr,fmt_rtp_efr: replace damaged packets with silence
......................................................................
Patch Set 3:
(3 comments)
File src/fmt_rtp_amr.c:
https://gerrit.osmocom.org/c/gapk/+/32094/comment/5e4eeea0_d1879047
PS2, Line 30: static const uint8_t SILENCE[] = {0x80, 0x44, 0x00, 0x00, 0x00, 0x00, 0x00};
> can you add a some comment with some spec reference as from where this hexstring comes from?
Ack
File src/fmt_rtp_efr.c:
https://gerrit.osmocom.org/c/gapk/+/32094/comment/81820468_713af606
PS2, Line 33: static const uint8_t DAMAGED_PACKET[EFR_LEN] = {
> Same here.
Ack
https://gerrit.osmocom.org/c/gapk/+/32094/comment/2f0f3bfc_a952cfe2
PS2, Line 70: if (!memcmp(DAMAGED_PACKET, src, EFR_LEN))
> is EFR_LEN garanteed to be == src_len here?
Yes, the if statement in line 65 checks it
--
To view, visit https://gerrit.osmocom.org/c/gapk/+/32094
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: gapk
Gerrit-Branch: master
Gerrit-Change-Id: I7245aa0bc0955cc8b94d5401a15e694f50498093
Gerrit-Change-Number: 32094
Gerrit-PatchSet: 3
Gerrit-Owner: matanp <matan1008(a)gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Mar 2023 13:57:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-gprs/+/32101
to look at the new patch set (#2).
Change subject: gmm: Several fixes to GMMSM prim alloc functions
......................................................................
gmm: Several fixes to GMMSM prim alloc functions
Change-Id: Ie49cb448806101a24c58a85f7073e13baccada5b
---
M include/osmocom/gprs/gmm/gmm_prim.h
M src/gmm/gmm_prim.c
2 files changed, 17 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/01/32101/2
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/32101
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: Ie49cb448806101a24c58a85f7073e13baccada5b
Gerrit-Change-Number: 32101
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-MessageType: newpatchset
Attention is currently required from: matanp.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/gapk/+/32094 )
Change subject: fmt_rtp_amr,fmt_rtp_efr: replace damaged packets with silence
......................................................................
Patch Set 3:
(1 comment)
File src/fmt_rtp_efr.c:
https://gerrit.osmocom.org/c/gapk/+/32094/comment/ec56af19_a7992598
PS2, Line 70: if (!memcmp(DAMAGED_PACKET, src, EFR_LEN))
> is EFR_LEN garanteed to be == src_len here?
Did you verify this already? maybe add an OSMO_ASSERT(src_len == EFR_LEN)?
--
To view, visit https://gerrit.osmocom.org/c/gapk/+/32094
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: gapk
Gerrit-Branch: master
Gerrit-Change-Id: I7245aa0bc0955cc8b94d5401a15e694f50498093
Gerrit-Change-Number: 32094
Gerrit-PatchSet: 3
Gerrit-Owner: matanp <matan1008(a)gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: matanp <matan1008(a)gmail.com>
Gerrit-Comment-Date: Tue, 28 Mar 2023 13:46:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge.
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32093 )
Change subject: Replace explicit gsm_lchan_name() calls with LOGPLCHAN
......................................................................
Patch Set 2:
(4 comments)
File src/common/rsl.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5518):
https://gerrit.osmocom.org/c/osmo-bts/+/32093/comment/3096b769_87c64101
PS2, Line 3298: gsm_pchan_name(ts->pchan), pdch_act? "PDCH" : "TCH/F", ts->flags);
spaces required around that '?' (ctx:VxW)
File src/osmo-bts-lc15/oml.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5518):
https://gerrit.osmocom.org/c/osmo-bts/+/32093/comment/50b391c0_84c50973
PS2, Line 937: LOGPLCHAN(lchan, DL1C, LOGL_INFO, "%s tch_mode=0x%02x\n", __FUNCTION__, lchan->tch_mode);
__func__ should be used instead of gcc specific __FUNCTION__
File src/osmo-bts-oc2g/oml.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5518):
https://gerrit.osmocom.org/c/osmo-bts/+/32093/comment/38cfaac0_3ca25dd9
PS2, Line 952: LOGPLCHAN(lchan, DL1C, LOGL_INFO, "%s tch_mode=0x%02x\n", __FUNCTION__, lchan->tch_mode);
__func__ should be used instead of gcc specific __FUNCTION__
File src/osmo-bts-sysmo/oml.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5518):
https://gerrit.osmocom.org/c/osmo-bts/+/32093/comment/fd999cc7_96d926cd
PS2, Line 945: __FUNCTION__, lchan->tch_mode);
__func__ should be used instead of gcc specific __FUNCTION__
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32093
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: If4f4f555f5ca61dfa624b298805f5375efc0b137
Gerrit-Change-Number: 32093
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Tue, 28 Mar 2023 13:43:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/32092
to look at the new patch set (#2).
Change subject: bts-{sysmo,oc2g,lc15}: Dump logical channel params during MPH-ACTIVATE.req
......................................................................
bts-{sysmo,oc2g,lc15}: Dump logical channel params during MPH-ACTIVATE.req
So far we only printed it during later modification. Let's print
it also during initial activation of a logical channel.
Change-Id: I6982a52905e4719e2e9c40630252ffef2ff9fbed
---
M src/osmo-bts-lc15/oml.c
M src/osmo-bts-oc2g/oml.c
M src/osmo-bts-sysmo/oml.c
3 files changed, 21 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/92/32092/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32092
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I6982a52905e4719e2e9c40630252ffef2ff9fbed
Gerrit-Change-Number: 32092
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
Hello Jenkins Builder, pespin, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/32088
to look at the new patch set (#4).
Change subject: bts-{sysmo,oc2g,lc15}: Fix RTP of AMR SID_FIRST_P1
......................................................................
bts-{sysmo,oc2g,lc15}: Fix RTP of AMR SID_FIRST_P1
When we receive a SID_FIRST_P1 frame from the PHY (during AMR/HR DTXu),
we must generate a SID frame on the RTP side.
The existing code
* ignored that the Amr_SidFirstP1 PHYIF message actually contains the
RTP payload
* manually generated the same content using osmo_amr_rtp_enc()
* forgot to prefix that with the AMR CMR+TOC
* in the end, sent a completely broken (too short) AMR SID frame over RTP
Let's fix this by simply using the 7-byte RTP payload with CMR+TOC that
the PHY is providing to us.
Change-Id: I90479efcc002d497648a71e73847af54e6208358
Related: OS#5944
---
M src/osmo-bts-lc15/tch.c
M src/osmo-bts-oc2g/tch.c
M src/osmo-bts-sysmo/tch.c
3 files changed, 29 insertions(+), 24 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/88/32088/4
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32088
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I90479efcc002d497648a71e73847af54e6208358
Gerrit-Change-Number: 32088
Gerrit-PatchSet: 4
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge.
Hello Jenkins Builder, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/32091
to look at the new patch set (#2).
Change subject: common/vty: Print AMR MultiRate Configuration in "show lchan"
......................................................................
common/vty: Print AMR MultiRate Configuration in "show lchan"
It can be useful to introspect the current active AMR mode set.
Related: OS#5944
Change-Id: I630af8c3835c2fcbea325c07db269d25e4e18f62
---
M src/common/vty.c
M src/osmo-bts-virtual/Makefile.am
2 files changed, 28 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/91/32091/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32091
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I630af8c3835c2fcbea325c07db269d25e4e18f62
Gerrit-Change-Number: 32091
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/32103 )
Change subject: lc15: fix compiler warning about wrong indent
......................................................................
lc15: fix compiler warning about wrong indent
l1_if.c: In function ‘activate_rf_compl_cb’:
l1_if.c:1280:17: error: this ‘if’ clause does not guard... [-Werror=misleading-indentation]
1280 | if (bts_lc15->led_ctrl_mode == LC15_LED_CONTROL_BTS)
| ^~
In file included from ../../include/osmo-bts/dtx_dl_amr_fsm.h:3,
from ../../include/osmo-bts/msg_utils.h:8,
from l1_if.c:55:
l1_if.c:1282:25: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
1282 | osmo_fsm_inst_dispatch(trx->mo.fi, NM_EV_DISABLE, NULL);
| ^~~~~~~~~~~~~~~~~~~~~~
Change-Id: I00ae3faf0f85fecf6e15e71dff071165725e547c
---
M src/osmo-bts-lc15/l1_if.c
1 file changed, 22 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/03/32103/1
diff --git a/src/osmo-bts-lc15/l1_if.c b/src/osmo-bts-lc15/l1_if.c
index 2955440..7af3be6 100644
--- a/src/osmo-bts-lc15/l1_if.c
+++ b/src/osmo-bts-lc15/l1_if.c
@@ -1279,8 +1279,8 @@
} else {
if (bts_lc15->led_ctrl_mode == LC15_LED_CONTROL_BTS)
bts_update_status(BTS_STATUS_RF_ACTIVE, 0);
- osmo_fsm_inst_dispatch(trx->mo.fi, NM_EV_DISABLE, NULL);
- osmo_fsm_inst_dispatch(trx->bb_transc.mo.fi, NM_EV_DISABLE, NULL);
+ osmo_fsm_inst_dispatch(trx->mo.fi, NM_EV_DISABLE, NULL);
+ osmo_fsm_inst_dispatch(trx->bb_transc.mo.fi, NM_EV_DISABLE, NULL);
}
msgb_free(resp);
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32103
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I00ae3faf0f85fecf6e15e71dff071165725e547c
Gerrit-Change-Number: 32103
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newchange