History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: XCL-115
Type: Bug Bug
Status: Open Open
Priority: Major Major
Assignee: Unassigned
Reporter: Kwint
Votes: 0
Watchers: 3
Operations

If you were logged in you would be able to see more operations.
sipXtapi: sipXcallLib

sipxConferenceJoin call handle

Created: 2006-09-19 08:22   Updated: 2007-04-26 15:20
Component/s: sipXtapi
Affects Version/s: 3.0
Fix Version/s: None

Original Estimate: Unknown Remaining Estimate: Unknown Time Spent: Unknown
File Attachments: 1. File ConferenceSplit.diff (3 kb)
2. File sipXtapi.diff (15 kb)
3. File sipXtapiInternalCpp.diff (0.9 kb)
4. File sipXtapiInternalH.diff (0.6 kb)



 Description  « Hide
When sipxConferenceJoin method is called, it overwrites pCallData->callId of the
call. So, when the sipxCallLookupHandle is called from sipxFireCallEvent with
the old callId, method cannot lookup right call handle... So the second call (not
the conference shell) doesn't fire any DISCONNECTED or DESTROYED events :(

To fix that i've added one more field to SIPX_CONF_DATA: oldCallId, save old callId in the sipxConferenceJoin, and check it in the sipxCallLookupHandle

 All   Comments   Work Log   Change History      Sort Order:
Jaroslav Libak - 2007-03-09 18:22
This is from mailinglist:

If you have a conference with 2 or more participants, and use split function in sipxtapi, you can't unhold it and you don't get any events.

1.) You cant unhold if there are no calls left in conference after split, because sessionCallId used in sipxtapi gets overwritten with wrong value (obviously with just plain CallId of conference which is wrong), and sipxCallGetCommonData will return this wrong value to unhold when asked for CallId (it prefers to use sessionId if its present, if not then CallId).
SessionCallId is Id used in sipxtacklib. It is set for both inbound and outbound calls.

Since we get wrong CallId from sipxCallGetCommonData , CallManager is sent also wrong CallId, and hold cannot succeed as we find wrong CpPeerCall which doesnt have a connection (it is the CpPeerCall of the conference which is empty). If hold is attempted after split and there is still a call left in the conference, then hold will be invoked on the wrong call - on the one in the conference not the one that was split.

Also please fix the manual. It says that after sipxConferenceSplit, call will remain in held state, this is not true, if its the only call then it will be unheld, this is what pCallData->pInst->pCallManager->createCall(&targetCallId) ; does by default in sipxConferenceSplit.

2.) No events are received - this is because in sipxtapi we register a handler in sipxtacklib, which sends us call events with the sessionCallId. Therefore sessionCallId of a call shouldnt be overwritten.

If this was incomprehensible, then the problem was that somebody was mixing sessionCallId and plain CallId in sipxtapi. They are not the same and shouldnt be swapped. sessionCallId is used in sipxtacklib and identifies SipConnection or Connection (which is parent of SipConnection). Plain CallId identifies CpPeerCall, we could also say the flow graph (as CpPeerCall has only 1 flowgraph). CpPeerCall can have multiple Connections and single flow graph with multiple media connections in case of a conference call. sipXtapi hold operation works by sending a message to sipxcalllib with a CallId/sessionCallId. It doesnt matter which one you send, as it asks CpPeerCall whether it has that CallId and it replies yes if the CallId of CpPeerCall equals or the sessionCallId (where it is called connectionCallId) of Connection equals.

Attached patch solves described problem with conferences (and XCL-115 <http://track.sipfoundry.org/browse/XCL-115&gt;), and maybe some more. I tested it with 1 call, 2 calls in coference, added by the sipxtapi add function or join. After that I used split and watched whether I get the events and can hold/unhold the call.

The patch basically keeps the old sessionCallId of the split call, as there is no reason to change it since SipConnection isn't closed.
Moreover it changes way how CallIds are assigned in the sipxConferenceAdd. This is to fix some other bugs with calls having no sessionCallId and thus getting no events. It should be noted that calls in a conference can happily have the same CallId, it isnt a problem at all, but they must have different sessionCallId. When a call is split, we generate new CallId for it, sessionCallId stays the same so its ok.

This patch probably won't apply cleanly, it might complain about a damaged patch or something. It is because I had to edit it by hand (I can no longer provide clean patches for some files, as I have too many patches in them). If it doesnt apply cleanly, just delete the section like "@@ -3371,9 +3379,18 @@" from the patch that was applied successfuly and try again.

Jaroslav Libak

Jaroslav Libak - 2007-03-14 13:15
I have identified 2 more problems that still exist even after this patch:

1.) When the last call is removed from conference, by putting conference on hold, also call that is outside conference is put on hold.
2.) Under rare conditions no events for a call split for conference are received. I dont know the exact steps that lead to this bug, but it occurs when multiple simultaneous conferences are created and calls are added and split.

Jaroslav Libak - 2007-03-15 16:06
This patch fixes problem 1. It was actually caused by the fact that after the last call is removed from conference, the conference shell CpPeerCall remains and can be unheld. This means we can give focus to it even though it has no connections. Other active call would thus be changed to bridged state if we unheld such conference.

problem 2. is caused by a problem in netintask if TURN is enabled. Winsock select reports a socket is ready for reading, but when we later try to receive data from it, it turns out recvfrom function blocks, thus blocking whole netintask and other threads sending commands to netintask.

Jaroslav Libak - 2007-03-15 16:37
Im attatching another missing diff.
It changes strCallId from pointer into variable. I prefer this to UtlString pointer craziness in structures. Why can't we have just UtlString everywhere and pass by const reference?

Jaroslav Libak - 2007-03-15 16:53
Another missing diff.

Jaroslav Libak - 2007-03-15 18:53
I identified another problem. If call has remote origin, and is added to conference and then split, attempt to put it on hold will result in crash. I will investigate why.

Jaroslav Libak - 2007-04-26 15:20
Patches are applied, the last mentioned problem remains. That one will be fixed when I review the whole conferencing code. It is caused the fact that for inbound call, we only know the sessionCallId (= sip call id) from sipxtacklib and not the CallId (this one starts with c).