March 2023 - gerrit-log - lists.osmocom.org

[S] Change in gapk[master]: fmt_rtp_amr,fmt_rtp_efr: replace damaged packets with silence
by Matan Perelman 28 Mar '23

28 Mar '23

Matan Perelman has uploaded this change for review. ( https://gerrit.osmocom.org/c/gapk/+/32094 ) Change subject: fmt_rtp_amr,fmt_rtp_efr: replace damaged packets with silence ...................................................................... fmt_rtp_amr,fmt_rtp_efr: replace damaged packets with silence Change-Id: I7245aa0bc0955cc8b94d5401a15e694f50498093 --- M src/fmt_rtp_amr.c M src/fmt_rtp_efr.c 2 files changed, 30 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/gapk refs/changes/94/32094/1 diff --git a/src/fmt_rtp_amr.c b/src/fmt_rtp_amr.c index a6a3c9f..02d74ca 100644 --- a/src/fmt_rtp_amr.c +++ b/src/fmt_rtp_amr.c @@ -27,6 +27,8 @@ #include <osmocom/gapk/formats.h> #include <osmocom/gapk/utils.h> +static const uint8_t SILENCE[] = {0x80, 0x44, 0x00, 0x00, 0x00, 0x00, 0x00}; + /* conversion function: RTP payload -> canonical format */ static int rtp_amr_from_canon(uint8_t *dst, const uint8_t *src, unsigned int src_len) @@ -42,6 +44,12 @@ static int rtp_amr_to_canon(uint8_t *dst, const uint8_t *src, unsigned int src_len) { + /* Detect severely damaged frames according to RFC4867 4.3.2 */ + if ((src[1] & 0x04) == 0) { + src = SILENCE; + src_len = sizeof(SILENCE); + } + /* skip Payload Header according to RFC4867 4.4.1 */ memcpy(dst, src+1, src_len-1); diff --git a/src/fmt_rtp_efr.c b/src/fmt_rtp_efr.c index 7afb1a3..34b8823 100644 --- a/src/fmt_rtp_efr.c +++ b/src/fmt_rtp_efr.c @@ -30,6 +30,15 @@ #define EFR_LEN 31 #define EFR_MAGIC 0xc +static const uint8_t DAMAGED_PACKET[EFR_LEN] = { + 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 +}; +static const uint8_t SILENCE[EFR_LEN] = { + 0xcf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xe0, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 +}; + /* conversion function: RTP payload -> canonical format */ static int rtp_efr_from_canon(uint8_t *dst, const uint8_t *src, unsigned int src_len) @@ -58,6 +67,10 @@ return EFR_CANON_LEN; } + if (!memcmp(DAMAGED_PACKET, src, EFR_LEN)) { + src = SILENCE; + } + for (i=0; i<(EFR_LEN-1); i++) dst[i] = (src[i] << 4) | (src[i+1] >> 4); dst[EFR_LEN-1] = src[EFR_LEN-1] << 4; -- 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: 1 Gerrit-Owner: Matan Perelman <matan1008(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in osmo-msc[master]: coverity: sdp_msg_test.c: check rc of sdp_msg_from_sdp_str()
by neels 28 Mar '23

28 Mar '23

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/31864 ) Change subject: coverity: sdp_msg_test.c: check rc of sdp_msg_from_sdp_str() ...................................................................... coverity: sdp_msg_test.c: check rc of sdp_msg_from_sdp_str() Related: CID#310975 Change-Id: I69d8972c58fb7cde965ff0b231947dd26247c2c7 --- M tests/sdp_msg/sdp_msg_test.c 1 file changed, 11 insertions(+), 1 deletion(-) Approvals: fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/tests/sdp_msg/sdp_msg_test.c b/tests/sdp_msg/sdp_msg_test.c index a1ffac0..ebe8328 100644 --- a/tests/sdp_msg/sdp_msg_test.c +++ b/tests/sdp_msg/sdp_msg_test.c @@ -136,7 +136,7 @@ printf("\n[%d]\n", i); dump_sdp(t->sdp_input, "sdp input: "); - sdp_msg_from_sdp_str(&sdp, t->sdp_input); + OSMO_ASSERT(sdp_msg_from_sdp_str(&sdp, t->sdp_input) == 0); sdp_msg_to_sdp_str_buf(str, sizeof(str), &sdp); dump_sdp(str, "sdp_msg_to_sdp_str_buf: "); -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/31864 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I69d8972c58fb7cde965ff0b231947dd26247c2c7 Gerrit-Change-Number: 31864 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in libosmocore[master]: logging: add timestamp fmt 'date' = 2018-01-16,01:44:34.681
by neels 28 Mar '23

28 Mar '23

Attention is currently required from: laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/5815 ) Change subject: logging: add timestamp fmt 'date' = 2018-01-16,01:44:34.681 ...................................................................... Patch Set 7: This change is ready for review. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/5815 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Icbd5192ea835e24b12fe057cc1ab56e9572d75c0 Gerrit-Change-Number: 5815 Gerrit-PatchSet: 7 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Tue, 28 Mar 2023 03:54:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: logging vty: add 'logging print timestamp', deprecate other timestamp...
by neels 28 Mar '23

28 Mar '23

neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/5861 ) Change subject: logging vty: add 'logging print timestamp', deprecate other timestamp cmds ...................................................................... Patch Set 4: This change is ready for review. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/5861 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I58c792dda3cbcf8618648ba4429c27fa398a9e15 Gerrit-Change-Number: 5861 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Mar 2023 03:54:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: logging: use enum to set timestamp format
by neels 28 Mar '23

28 Mar '23

Attention is currently required from: laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/5857 ) Change subject: logging: use enum to set timestamp format ...................................................................... Patch Set 6: This change is ready for review. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/5857 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I70ecee543c266df54191d3e86401466cd1a5c0a6 Gerrit-Change-Number: 5857 Gerrit-PatchSet: 6 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Tue, 28 Mar 2023 03:54:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: logging: add 'logging timezone (localtime|utc)'
by neels 28 Mar '23

28 Mar '23

Attention is currently required from: laforge, fixeria, pespin. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/32043 to look at the new patch set (#2). Change subject: logging: add 'logging timezone (localtime|utc)' ...................................................................... logging: add 'logging timezone (localtime|utc)' Add ability to print logging timestamps in UTC. I want to test logging timestamps in vty transcript tests. To get deterministic timestamps, I can use osmo_gettimeofday_override(). However, so far we log in the server's local time zone: the dates printed after osmo_gettimeofday_override() still change according to the locally configured timezone -- not good for regression testing. For UTC, it is always the same. The intended use is for regression testing of timestamps, but maybe logging in UTC may be useful to some user out there, too. Change-Id: I7f868b47bf8f8dfcf85e735f490ae69b18111af4 --- M configure.ac M include/osmocom/core/logging.h M src/core/libosmocore.map M src/core/logging.c M src/vty/logging_vty.c M tests/logging/logging_vty_test.vty 6 files changed, 118 insertions(+), 12 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/43/32043/2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32043 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I7f868b47bf8f8dfcf85e735f490ae69b18111af4 Gerrit-Change-Number: 32043 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in libosmocore[master]: logging: add timestamp fmt 'date' = 2018-01-16,01:44:34.681
by neels 28 Mar '23

28 Mar '23

neels has removed a vote from this change. ( https://gerrit.osmocom.org/c/libosmocore/+/5815 ) Change subject: logging: add timestamp fmt 'date' = 2018-01-16,01:44:34.681 ...................................................................... Removed Code-Review-2 by neels <nhofmeyr(a)sysmocom.de> -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/5815 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Icbd5192ea835e24b12fe057cc1ab56e9572d75c0 Gerrit-Change-Number: 5815 Gerrit-PatchSet: 6 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: deleteVote

1 0

[S] Change in libosmocore[master]: logging_vty.c: rename static function
by neels 28 Mar '23

28 Mar '23

Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32045 ) Change subject: logging_vty.c: rename static function ...................................................................... Patch Set 1: (1 comment) File src/vty/logging_vty.c: https://gerrit.osmocom.org/c/libosmocore/+/32045/comment/b7c8a626_3c6c1ef3 PS1, Line 193: logging_prnt_timezone, : logging_timezone_cmd, > Ack how th did that happen, thx -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32045 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I5a651e393bd438ea236659ed2c86188a8f9885d1 Gerrit-Change-Number: 32045 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Mar 2023 03:46:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: logging: add 'logging timezone (localtime|utc)'
by neels 28 Mar '23

28 Mar '23

Attention is currently required from: laforge, fixeria, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32043 ) Change subject: logging: add 'logging timezone (localtime|utc)' ...................................................................... Patch Set 1: (8 comments) Patchset: PS1: > I would probably have used a bool rather than an enum as I see no chance we ever want to deal with a […] yes but i figured after converting lots of bools to enums, it would be weird to go on to add a new bool again in something that *could* conceptually have more than two preferences... PS1: (new patch set coming up) File include/osmocom/core/logging.h: https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/2d045bdd_9b9fe8af PS1, Line 400: /* Set timezone for logging of timestamps: 0 for local time, 1 for UTC. Other values are reserved for future > 0 and 1? don't we have enums for that in this patch? right, thx File src/core/logging.c: https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/7cf368c8_78c510f5 PS1, Line 474: return 0; > I'd really return an error if the requested timestamp cannot be obtained. the code before this patch didn't care about the return value of localtime_r(), but if you insist... https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/a14a672d_6516ed7b PS1, Line 528: } else if (target->print_timestamp && get_timestamp(&tv, &tm, target) == 0) { > you are calling get_timestamp twice here every time we log something. […] There is an unlikely corner case where it gets called twice, i.e. only if calling the first time failed. In all practical cases it is called once. This code changes to a cleaner switch() in upcoming https://gerrit.osmocom.org/c/libosmocore/+/5857/5/src/core/logging.c#520 https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/f17936bf_4162a716 PS1, Line 874: target->timezone = timezone; > Ack i don't agree. i would not want to duplicate some enum validation code here. It can be assumed that the caller only passes valid enum values, and if it is an unknown enum value, then the logging.c switch() deals with that. Otherwise we always have to keep this function "artificially" updated with what logging.c supports. File src/vty/logging_vty.c: https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/89946725_1c316042 PS1, Line 203: log_set_timezone(tgt, LOG_TIMEZONE_UTC); > Ack if i can defend above argument, then no here as well =) File tests/logging/logging_vty_test.vty: https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/5058ecde_1a21934f PS1, Line 75: logging_vty_test# logging timestamp ? > See https://gerrit.osmocom.org/c/libosmocore/+/32044. pau has a solid point here, i failed to add usability tests for the 'logging timestamp' command itself, which now unconvered that i so far fail to write back the new vty command. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32043 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I7f868b47bf8f8dfcf85e735f490ae69b18111af4 Gerrit-Change-Number: 32043 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 28 Mar 2023 03:27:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: logging vty: add 'logging print timestamp', deprecate other timestamp...
by neels 28 Mar '23

28 Mar '23

neels has removed a vote from this change. ( https://gerrit.osmocom.org/c/libosmocore/+/5861 ) Change subject: logging vty: add 'logging print timestamp', deprecate other timestamp cmds ...................................................................... Removed Code-Review-2 by neels <nhofmeyr(a)sysmocom.de> -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/5861 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I58c792dda3cbcf8618648ba4429c27fa398a9e15 Gerrit-Change-Number: 5861 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: deleteVote

1 0