> On 23 Mar 2016, at 19:14, [email protected] wrote:
> 
> From: Max <[email protected]>
> 
> Note: ideally this situation should not happen - we should check
> channel compatibility before paging 2nd leg of the call.
> 
> 
> 
> +     if (lt != rt) {


lt / rt is not declared in this patch (and yes, it belongs into this one. But 
channel type is still not good enough. Let's assume I have a TCH/H and a TCH/F 
and I use AMR5.9 on both of these channels. Then the voice codec (and its 
parameters) are compatible.

I don't know the specific ticket and ultimate goal but I think either you check 
for codec compatibility or change the wording to refer only to channel type and 
leave the actual voice codec incompat for another day.





> +             LOGP(DCC, LOGL_ERROR, "Cannot patch through call with different"
> +                  " channel types: local = %s, remote = %s\n",
> +                  osmo_gsm48_chan_type2str(lt), 
> osmo_gsm48_chan_type2str(rt));
> +             return -EBADSLT;
> +     }
> +
>       // todo: map between different bts types
>       switch (bts->type) {
>       case GSM_BTS_TYPE_NANOBTS:
> @@ -1851,6 +1858,26 @@ static void gsm48_cc_timeout(void *arg)
> 
> }
> 
> +static inline void disconnect_bridge(struct gsm_network *net,
> +                                 struct gsm_mncc_bridge *bridge)
> +{
> +     struct gsm_trans *trans0 = trans_find_by_callref(net, 
> bridge->callref[0]);
> +     struct gsm_trans *trans1 = trans_find_by_callref(net, 
> bridge->callref[1]);
> +     struct gsm_mncc mx_rel;
> +     if (!trans0 || !trans1)
> +             return;


Can you please elaborate about the intention of this method? You try to undo a 
failed mapping and do this by disconnecting the call? Is that the right thing 
to do? Has there been any side effect by the call of tch_map?

In the long run should there be a MNCC_BRIDGE_REJ answer to the MNCC_BRIDGE 
call?


> @@ -3220,7 +3247,12 @@ int mncc_tx_to_cc(struct gsm_network *net, int 
> msg_type, void *arg)
>       /* handle special messages */
>       switch(msg_type) {
>       case MNCC_BRIDGE:
> -             return tch_bridge(net, arg);
> +             rc = tch_bridge(net, arg);
> +             if (rc < 0) {
> +                     DEBUGP(DCC, "Failed to bridge TCH: %s\n", 
> strerror(-rc));

Do you think it makes sense to include both callrefs?

holger

Reply via email to