Attention is currently required from: dexter.
6 comments:
Commit Message:
Patch Set #8, Line 12: againt
agent?
Patchset:
I'm having difficulties reviewing: I need to manually figure out whether mgcp_codec_decide() has changed. Is it possible / easy to add a separate patch that only moves the function, and then I can easily see here what parts of it changed?
I wrote some review below but I haven't completely read and understood this patch yet. Would be great if you could clarify a bit, thanks!
File src/libosmo-mgcp/mgcp_codec.c:
codecs_assigned = rtp_end->codecs_assigned;
OSMO_ASS
(lets put these two lines inside the 'if { ...' below)
(add a comma "in case of foo, barize")
Patch Set #8, Line 401: * a tentative decision is made.
i fail to understand:
1) Decide for both sides? The MGW has two lists of pt-nr to codec. It gets a specific pt-nr in an RTP packet on one side, and needs to decide what the pt-nr for this codec is on the other side. So how can there be a decision on both sides?
2) destination connection? RTP is UDP, there is no connection? do you mean to say "in case no destination endpoint is set up"?
3) how do codec choices matter when there is no destination side? this only makes sense to run when both sides of RTP are set up and ready?
File src/libosmo-mgcp/mgcp_network.c:
Patch Set #8, Line 511: rtp_hdr->payload_type = (uint8_t) conn_dst->end.codec->payload_type;
this looks wrong. There should not be one specific payload type on an endpoint. There should always be a list of possible payload types.
This function should get as input the payload type number of one specific RTP packet that arrived, plus the list of pt-to-codec from the src side, and also the destination endpoint.
1. This should look up the codec that src-pt-nr means, in the source endpoint cfg.
2. Then should find this codec in the destination endpoint list and patch the matching pt-nr into this particular RTP packet.
What confuses me is that it seems this is what the function was doing before this patch, and now it is changed to sort of pick one specific codec from conn_dst->end that has been decided on beforehand? (which would be wrong AFAIK)
To view, visit change 32218. To unsubscribe, or for help writing mail filters, visit settings.