Attention is currently required from: pespin.

dexter has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/34349?usp=email )

Change subject: mgcp_client_fsm: allow the same codec multiple times in ptmap
......................................................................


Patch Set 3:

(4 comments)

Commit Message:

https://gerrit.osmocom.org/c/osmo-mgw/+/34349/comment/0bb31340_ea6347a7
PS2, Line 10: he which to used and optionally a payload type map (ptmap) that 
is used
> "he which" makes no sense :)
Done


Patchset:

PS2:
> After looking at the whole patch, I have the feeling this patch is a HACK and 
> not a real fix. […]
The way how ptmap works is not changed by this patch. It still works as 
expected when used with unique codec/pt pairs. All we do now is to allow the 
same codec appearing multiple times under the assumption that the order matches 
to order we have in the codecs[] array. I think this is an elegant fix since it 
does not change the API interface. It only extends the way how ptmap is used a 
little bit. Since we do not have to change the struct in any way we do not 
break the ABI, nor the API.


File include/osmocom/mgcp_client/mgcp_client_fsm.h:

https://gerrit.osmocom.org/c/osmo-mgw/+/34349/comment/f73e3b0f_aedc8b71
PS2, Line 39:    * codecs array above). In case the same codec type (enum 
mgcp_codecs) is appearing multiple times in codecs.
> This sentence is wrong and I'm not following it: "In case ... […]
Done


File src/libosmo-mgcp-client/mgcp_client.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/34349/comment/f61bce52_98b8cf60
PS2, Line 152: unsigned int map_codec_to_pt2(bool *ptmap_used, const struct 
ptmap *ptmap,
> I'd say better move the "ptmap_used" arg to the end, since it's the only 
> in-out param (so when readi […]
Are you sure? Our convention is to have the output parameters first and then 
everything that is input. Or do I miss something here?



--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34349?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ie13ce59d3165936a16e16588b4d58f0ce7e0ae67
Gerrit-Change-Number: 34349
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Comment-Date: Wed, 13 Sep 2023 09:28:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <[email protected]>
Gerrit-MessageType: comment

Reply via email to