This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
neels gerrit-no-reply at lists.osmocom.orgneels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/15953 ) Change subject: add full SDP codec information to the MNCC socket ...................................................................... Patch Set 6: (5 comments) https://gerrit.osmocom.org/c/osmo-msc/+/15953/5//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/c/osmo-msc/+/15953/5//COMMIT_MSG@7 PS5, Line 7: add full SDP codec information to the MNCC socket > So there's 2 main topics here which seem would be in different commits, splitting this huge commit: […] That is true, but it is also true that it is realitvely hard to do one without the other. I actually started out separating it once, and I gave up, because it seemed to be too much effort. The reasons: to only move the CRCX implies changing the order of the callbacks between msc_a and gsm_04_08_cc. That semantically separates in different places than before, and would require keeping state across those calls. Keeping that state is in this patch implemented by cc_sdp.c, which accumulates all information about what parts imply what codec choices. So, "just for the sake of review", I would invent a different patch version, that keeps state in a way that needs to be made to work and make sense, yet is again completely discarded one patch later. I would argue that reviewers should in fact take a test setup, be it in hardware or in ttcn3, and examine what this code does and how it acts and reacts. This kind of profound change should anyway see such scrutiny, IMHO. To put it in blunt words, I want people to understand this patch and what it is doing. Now, OTOH, that may be naive. I am not sure what benefit it would bring to do the extra work and separate patches, just to get two half uninformed reviews instead of no informed review at all. But if it means getting the patch merged (and if sysmocom agrees in the form of time available) then I can also invest more time in separating this paradigm shift into one weird patch that is then followed by still a paradigm shift. I know I'm sounding sarcastic, but whatever the community needs is fine with me. Maybe it does need to sit here a bit longer, maybe needs to be test-run at congress first, gain more audience before it can be merged. I personally think this change is really exciting, also with the other change that I still have in mind for after this patch, because it allows a truly informed codec choice for the first time. So we can let this sit here for a bit, I'm fairly confident that someone or other will be drawn to it at some point and give it deep attention. https://gerrit.osmocom.org/c/osmo-msc/+/15953/5/include/osmocom/msc/gsm_04_08.h File include/osmocom/msc/gsm_04_08.h: https://gerrit.osmocom.org/c/osmo-msc/+/15953/5/include/osmocom/msc/gsm_04_08.h@51 PS5, Line 51: int cc_assignment_done(struct gsm_trans *trans); (talking about these callbacks above) https://gerrit.osmocom.org/c/osmo-msc/+/15953/5/src/libmsc/gsm_04_08_cc.c File src/libmsc/gsm_04_08_cc.c: https://gerrit.osmocom.org/c/osmo-msc/+/15953/5/src/libmsc/gsm_04_08_cc.c@581 PS5, Line 581: &trans->msc_a->cc.codec_list_bss_supported); (talking about this state above) https://gerrit.osmocom.org/c/osmo-msc/+/15953/5/src/libmsc/gsm_04_08_cc.c@640 PS5, Line 640: sdp->rtp = *rtp_cn_local; (and this state) https://gerrit.osmocom.org/c/osmo-msc/+/15953/5/src/libmsc/gsm_04_08_cc.c@719 PS5, Line 719: sdp_audio_codecs_to_bearer_cap(&trans->bearer_cap, &trans->cc.sdp.result.audio_codecs); (this state ... and so on) -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/15953 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I8c3b2de53ffae4ec3a66b9dabf308c290a2c999f Gerrit-Change-Number: 15953 Gerrit-PatchSet: 6 Gerrit-Owner: neels <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de> Gerrit-Reviewer: pespin <pespin at sysmocom.de> Gerrit-Comment-Date: Thu, 12 Dec 2019 15:04:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin at sysmocom.de> Gerrit-MessageType: comment -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191212/a7ab8697/attachment.htm>