falconia has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/37285?usp=email )
Change subject: {de,en}code8_hr: fix totally broken functions ...................................................................... {de,en}code8_hr: fix totally broken functions The functions decode8_hr() and encode8_hr() in trau_frame.c (encoding and decoding HRv1 TRAU frames in 8 kbit/s format) are available via osmo_trau_frame_decode_8k() and osmo_trau_frame_encode() public APIs, but they are not currently used anywhere in Osmocom. (OsmoMGW-E1 has no working support for HR codec, and there is no support for HRv1-8k in the TRAU<->RTP layer until next patch in this series.) Study of the code reveals that these functions were totally broken prior to this change: * The movement of Dn bits between trau_bits and fr->d_bits used wrong offsets for some of the bits (and even missed one group of bits entirely in the encoding direction), producing garbled codec frames. * The filling of XCn bits in the encoding direction was likewise garbled by use of wrong offset. * When encoding TRAU-UL frames in this format, bit C5 (odd parity) was set incorrectly. * The order of bits in fr->crc_bits array (Osmocom internal) was opposite the order needed for osmo_crc8gen_check_bits() and osmo_crc8gen_set_bits() functions - contrast with HRv1-16k functions that use sensible bit order for this fr->crc_bits array. Given that this HRv1-8k mode within TRAU frame encoding and decoding API was totally broken and not used anywhere, there are no compatibility concerns with changing aspects of this API such as CRC bit order. Change-Id: I7cf0275f2ff212e001db38d7b090f222f292cdb0 --- M src/trau/trau_frame.c 1 file changed, 49 insertions(+), 11 deletions(-) Approvals: falconia: Looks good to me, approved fixeria: Looks good to me, but someone else must approve laforge: Looks good to me, but someone else must approve pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/trau/trau_frame.c b/src/trau/trau_frame.c index 0be005a..389495c 100644 --- a/src/trau/trau_frame.c +++ b/src/trau/trau_frame.c @@ -780,15 +780,14 @@ } /* CRC0 .. CRC2 */ - fr->crc_bits[2] = trau_bits[82]; - fr->crc_bits[1] = trau_bits[83]; - fr->crc_bits[0] = trau_bits[84]; + memcpy(fr->crc_bits, trau_bits + 73, 3); /* D45 .. D48 */ - memcpy(fr->d_bits + d_idx, trau_bits + 85, 4); + memcpy(fr->d_bits + d_idx, trau_bits + 76, 4); + d_idx += 4; /* D49 .. D111 */ - for (i = 10; i < 10 + 10; i++) { + for (i = 10; i < 10 + 9; i++) { int offset = i * 8; memcpy(fr->d_bits + d_idx, trau_bits + offset + 1, 7); d_idx += 7; @@ -843,7 +842,7 @@ cbits_out[1] = 0; cbits_out[2] = 0; cbits_out[3] = 1; - cbits_out[4] = 1; + cbits_out[4] = 0; } else { cbits_out[0] = 0; cbits_out[1] = 0; @@ -855,7 +854,7 @@ /* XC1 .. XC2 */ memcpy(trau_bits + 1 * 8 + 6, fr->xc_bits, 2); /* XC3 .. XC6 */ - memcpy(trau_bits + 2 * 8 + 2, fr->xc_bits, 4); + memcpy(trau_bits + 2 * 8 + 2, fr->xc_bits + 2, 4); /* D1 .. D2 */ memcpy(trau_bits + 2 * 8 + 6, fr->d_bits, 2); d_idx += 2; @@ -868,12 +867,14 @@ }; /* CRC0 .. CRC2 */ - trau_bits[82] = fr->crc_bits[2]; - trau_bits[83] = fr->crc_bits[1]; - trau_bits[84] = fr->crc_bits[0]; + memcpy(trau_bits + 73, fr->crc_bits, 3); + + /* D45 .. D48 */ + memcpy(trau_bits + 76, fr->d_bits + d_idx, 4); + d_idx += 4; /* D49 .. D111 */ - for (i = 10; i < 10 + 10; i++) { + for (i = 10; i < 10 + 9; i++) { int offset = i * 8; memcpy(trau_bits + offset + 1, fr->d_bits + d_idx, 7); d_idx += 7; -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/37285?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I7cf0275f2ff212e001db38d7b090f222f292cdb0 Gerrit-Change-Number: 37285 Gerrit-PatchSet: 1 Gerrit-Owner: falconia <fal...@freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia <fal...@freecalypso.org> Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de> Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-MessageType: merged