cellular-infrastructure
/
osmo-mgw
mirror of https://gerrit.osmocom.org/osmo-mgw
10
0
Fork 0
Code Issues Releases Wiki Activity
osmo-mgw/a5e4931fc2f67a21cd7db8a863b...

338 lines
12 KiB
Plaintext
Raw Blame History

{
"comments": [
{
"unresolved": true,
"key": {
"uuid": "dda23540_0adc1624",
"filename": "include/osmocom/mgcp/mgcp_codec.h",
"patchSetId": 2
},
"lineNbr": 16,
"author": {
"id": 1000074
},
"writtenOn": "2023-04-05T14:27:26Z",
"side": 1,
"message": "my understanding is that this decides which codec is used in one direction between conn A and conn B, hence between conn_src and conn_dst, am I correct?\nIf that\u0027s the case, naming the conns \"conn_src\" and \"conn_dst\" may be a lot clearer.",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "3ce0af72_57723ec1",
"filename": "include/osmocom/mgcp/mgcp_codec.h",
"patchSetId": 2
},
"lineNbr": 16,
"author": {
"id": 1000028
},
"writtenOn": "2023-04-06T11:25:57Z",
"side": 1,
"message": "Done",
"parentUuid": "dda23540_0adc1624",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "86ee1800_07089044",
"filename": "src/libosmo-mgcp/mgcp_codec.c",
"patchSetId": 2
},
"lineNbr": 440,
"author": {
"id": 1000074
},
"writtenOn": "2023-04-05T14:27:26Z",
"side": 1,
"message": "I wonder why do you need to call mgcp_codec_find_same() twice here, something looks wrong imho.",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "28f2c879_c1d1d047",
"filename": "src/libosmo-mgcp/mgcp_codec.c",
"patchSetId": 2
},
"lineNbr": 440,
"author": {
"id": 1000028
},
"writtenOn": "2023-04-06T11:25:57Z",
"side": 1,
"message": "The problem is that it is not possible to assign codec_conn_dst to conn_src-\u003eend.codec since then the pointer in conn_src-\u003eend.codec becomes stale when conn_dst is freed. So we just search once more to get the pointer from the list within conn_src",
"parentUuid": "86ee1800_07089044",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "eae3fd85_ed27682e",
"filename": "src/libosmo-mgcp/mgcp_codec.c",
"patchSetId": 2
},
"lineNbr": 440,
"author": {
"id": 1000074
},
"writtenOn": "2023-04-06T12:17:42Z",
"side": 1,
"message": "Ah I see what you mean, you are looking the codec in each conn list.\nNow the question I have is: why do we have codec structs holding the same data duplicated (allocated several times) for each connection?\nWouldn\u0027t it make more sense to have them defined in 1 global place and then have a pointer to that global struct here in all conns?\nI\u0027m not saying it has to be done in this commit, just pointing out some possibilities for improvement.",
"parentUuid": "28f2c879_c1d1d047",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "932b783e_30d440a2",
"filename": "src/libosmo-mgcp/mgcp_codec.c",
"patchSetId": 2
},
"lineNbr": 440,
"author": {
"id": 1000028
},
"writtenOn": "2023-04-06T14:05:17Z",
"side": 1,
"message": "The data is not duplicated. Its just that each endpoint holds two connections and each connections has a codec list. This is the list that had been issued by the call agent during CRCX/MDCX. We can not put both into one list since then we would lose the info which side supports what.",
"parentUuid": "eae3fd85_ed27682e",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "4578ee34_6e0a70aa",
"filename": "src/libosmo-mgcp/mgcp_codec.c",
"patchSetId": 2
},
"lineNbr": 440,
"author": {
"id": 1000074
},
"writtenOn": "2023-04-06T14:41:21Z",
"side": 1,
"message": "\u003e and each connections has a codec list.\n\nAnd each codec in the list is a different object in memory on each connection right?, hence it is duplicated.\n\n\u003e We can not put both into one list since then we would lose the info which side supports what.\n\nI\u0027m not saying we drop the per-conn list. But the per-conn list would contain pointers to a global structure containing the codec data.",
"parentUuid": "932b783e_30d440a2",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "abbd313c_4c481666",
"filename": "src/libosmo-mgcp/mgcp_codec.c",
"patchSetId": 2
},
"lineNbr": 449,
"author": {
"id": 1000074
},
"writtenOn": "2023-04-05T14:27:26Z",
"side": 1,
"message": "I wonder why do you need to call codec_find_convertible() twice here, something looks wrong imho.",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "563f1f24_4f257dd8",
"filename": "src/libosmo-mgcp/mgcp_codec.c",
"patchSetId": 2
},
"lineNbr": 449,
"author": {
"id": 1000028
},
"writtenOn": "2023-04-06T11:25:57Z",
"side": 1,
"message": "(same as above)",
"parentUuid": "abbd313c_4c481666",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "a17adc26_d74c2c31",
"filename": "src/libosmo-mgcp/mgcp_codec.c",
"patchSetId": 2
},
"lineNbr": 461,
"author": {
"id": 1000074
},
"writtenOn": "2023-04-05T14:27:26Z",
"side": 1,
"message": "IIUC you are finding/selecting/setting the same codec A-\u003eB and B-\u003eA here, but I wonder if that really makes sense?\nCouldn\u0027t A-\u003eB and B-\u003eA be different? I think so.",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "257caf90_c3a7843a",
"filename": "src/libosmo-mgcp/mgcp_codec.c",
"patchSetId": 2
},
"lineNbr": 461,
"author": {
"id": 1000028
},
"writtenOn": "2023-04-06T11:25:57Z",
"side": 1,
"message": "When we trying to find a convertible codec, then it indeed may be the case that A ends up with a different codec than B. But it will always be a compatible codec. What however is true is when you swap conn_src and conn_dst, then the result will be different but I see no problem with this, it would still be two codecs that are compatible.\n\nThis is also more a theoretical issue, normally one side (the leg pointing to the MSC) will assign exactly one codec.",
"parentUuid": "a17adc26_d74c2c31",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "9fd74e24_397860ea",
"filename": "src/libosmo-mgcp/mgcp_network.c",
"patchSetId": 2
},
"lineNbr": 514,
"author": {
"id": 1000074
},
"writtenOn": "2023-04-05T14:27:26Z",
"side": 1,
"message": "I see no lookup being done here anymore.",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "bf1d569b_dc6ce036",
"filename": "src/libosmo-mgcp/mgcp_network.c",
"patchSetId": 2
},
"lineNbr": 514,
"author": {
"id": 1000028
},
"writtenOn": "2023-04-06T11:25:57Z",
"side": 1,
"message": "There was even more that I have overlooked.",
"parentUuid": "9fd74e24_397860ea",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "9f97142b_40e3c482",
"filename": "src/libosmo-mgcp/mgcp_protocol.c",
"patchSetId": 2
},
"lineNbr": 809,
"author": {
"id": 1000074
},
"writtenOn": "2023-04-05T14:27:26Z",
"side": 1,
"message": "see, here we have a clear concept of conn_src (\"conn\" here) and conn_dst (returned by mgcp_find_dst_conn).",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "f870f6af_3802cde0",
"filename": "src/libosmo-mgcp/mgcp_protocol.c",
"patchSetId": 2
},
"lineNbr": 809,
"author": {
"id": 1000028
},
"writtenOn": "2023-04-06T11:25:57Z",
"side": 1,
"message": "Done",
"parentUuid": "9f97142b_40e3c482",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "be543e56_5bc3d460",
"filename": "src/libosmo-mgcp/mgcp_protocol.c",
"patchSetId": 2
},
"lineNbr": 816,
"author": {
"id": 1000074
},
"writtenOn": "2023-04-05T14:27:26Z",
"side": 1,
"message": "in here you are deciding what codec is used to forward/transmit from conn_src to conn_dst.",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "3ce68c38_04c74e72",
"filename": "src/libosmo-mgcp/mgcp_protocol.c",
"patchSetId": 2
},
"lineNbr": 816,
"author": {
"id": 1000028
},
"writtenOn": "2023-04-06T11:25:57Z",
"side": 1,
"message": "Done",
"parentUuid": "be543e56_5bc3d460",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": true,
"key": {
"uuid": "5eb573ba_7c4d3a95",
"filename": "src/libosmo-mgcp/mgcp_vty.c",
"patchSetId": 2
},
"lineNbr": 703,
"author": {
"id": 1000074
},
"writtenOn": "2023-04-05T14:27:26Z",
"side": 1,
"message": "Add vty_out telling the user that this command is deprecated and is a NO-OP, so that they can drop it from their config.",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
},
{
"unresolved": false,
"key": {
"uuid": "0f544b7f_291209de",
"filename": "src/libosmo-mgcp/mgcp_vty.c",
"patchSetId": 2
},
"lineNbr": 703,
"author": {
"id": 1000028
},
"writtenOn": "2023-04-06T11:25:57Z",
"side": 1,
"message": "Oh, looks like this has been forgotten on all other DEFUN_DEPRECATED functions as well.",
"parentUuid": "5eb573ba_7c4d3a95",
"revId": "a5e4931fc2f67a21cd7db8a863bf567a95f8ee0e",
"serverId": "035e6965-6537-41bd-912c-053f3cf69326"
}
]
}
View Git Blame Copy Permalink