January 2016 - h323plus - lists.packetizer.com

Issue with H.263 codecs set
by Iurii Gordiienko 01 Feb '16

01 Feb '16

Hi all, I need an advice - how to work with set of H.263 codecs. Basically, I have all H.263 codec types capability implemented: H.263, H.263-1998 and H.263-2000. All these capabilities uses same H245_VideoCapability::e_h263VideoCapability value for GetSubType function. In result my localCapabilities variable has all these 3 capabilities as well. When I receiving remote capabilities the remoteCapabilities variable is loading by H323Capabilities::H323Capabilities(const H323Connection & connection, const H245_TerminalCapabilitySet & pdu) constructor and the function uses code like this (simplified): H323Capability * capability = localCapabilities.FindCapability(pdu.m_capabilityTable[i].m_capability, capabilityNo, pdu); if (capability != NULL) { H323Capability * copy = (H323Capability *)capability->Clone(); copy->SetCapabilityNumber(capabilityNo); if (copy->OnReceivedPDU(pdu.m_capabilityTable[i].m_capability)) table.Append(copy); else delete copy; } The main issue - localCapabilities.FindCapability function returns any first H.263 Capability from it's table, no matter which particular H.263 remote Capability was used, it only checks MainType and SubType... In result if remote side send me, for example, H.263-1998, but I have H.263 and H.263-1998 - I will have H.263 in result... Now I'm trying to override Connection::OnReceivedCapabilitySet(const H245_TerminalCapabilitySet & pdu) function, extract H.263 codecs manually, store all these capabilities and update remoteCapabilities after, but may be exists a proper solution for this case? Thanks -- Iurii Gordiienko

3 4

Bug inH323Capabilities::Remove(H323Capability*)
by Iurii Gordiienko 31 Jan '16

31 Jan '16

Hi, I found a bug - H323Capabilities::Remove function will skip item when doing .RemoveAt operation. In result I have had a crash (in line if (set[outer][middle][inner].GetCapabilityNumber() == capabilityNumber)) for some sets of capabilities. I can't understand why the crash is occurs but in any way... Original code: void H323Capabilities::Remove(H323Capability * capability) { if (capability == NULL) return; PTRACE(3, "H323\tRemoving capability: " << *capability); unsigned capabilityNumber = capability->GetCapabilityNumber(); for (PINDEX outer = 0; outer < set.GetSize(); outer++) { for (PINDEX middle = 0; middle < set[outer].GetSize(); middle++) { for (PINDEX inner = 0; inner < set[outer][middle].GetSize(); inner++) { if (set[outer][middle][inner].GetCapabilityNumber() == capabilityNumber) { set[outer][middle].RemoveAt(inner); break; } } if (set[outer][middle].GetSize() == 0) set[outer].RemoveAt(middle); } if (set[outer].GetSize() == 0) set.RemoveAt(outer); } My fix: void H323Capabilities::Remove(H323Capability * capability) { if (capability == NULL) return; PTRACE(3, "H323\tRemoving capability: " << *capability); unsigned capabilityNumber = capability->GetCapabilityNumber(); for (PINDEX outer = 0; outer < set.GetSize(); ) { for (PINDEX middle = 0; middle < set[outer].GetSize(); ) { for (PINDEX inner = 0; inner < set[outer][middle].GetSize(); inner++) { if (set[outer][middle][inner].GetCapabilityNumber() == capabilityNumber) { set[outer][middle].RemoveAt(inner); break; } } if (set[outer][middle].GetSize()) ++middle; else set[outer].RemoveAt(middle); } if (set[outer].GetSize()) ++outer; else set.RemoveAt(outer); } Thanks -- Iurii Gordiienko

2 1

Double-delete Capability bug
by Iurii Gordiienko 25 Jan '16

25 Jan '16

Hi, I have found something... For H323ExtendedVideoCapability::AddAllCapabilities function we are using H323CodecExtendedVideoCapability::AddCapability function for extCapability->AddCapability(*r). H323CodecExtendedVideoCapability::AddCapability function uses extCapabilities.Add(H323ExtendedVideoFactory::CreateInstance(cap)) function. The function H323ExtendedVideoFactory::CreateInstance(cap) will call "new" if any same "cap" does not is exists for now< for other case it will return the pointer to current actual Capability. But H323CodecExtendedVideoCapability::extCapabilities will delete all items in time of execution extCapabilities.RemoveAll() function from H323CodecExtendedVideoCapability::~H323CodecExtendedVideoCapability(). In result we have "double delete" and undefined behavior (crash for worst case). -------------------------------- void H323ExtendedVideoCapability::AddAllCapabilities( H323Capabilities & basecapabilities, PINDEX descriptorNum,PINDEX simultaneous) { H323ExtendedVideoFactory::KeyList_T extCaps = H323ExtendedVideoFactory::GetKeyList(); if (extCaps.size() > 0) { H323CodecExtendedVideoCapability * capability = new H323CodecExtendedVideoCapability(); H323ExtendedVideoFactory::KeyList_T::const_iterator r; PINDEX num = P_MAX_INDEX; for (r = extCaps.begin(); r != extCaps.end(); ++r) { H323CodecExtendedVideoCapability * extCapability = (H323CodecExtendedVideoCapability *)capability->Clone(); extCapability->AddCapability(*r); num = basecapabilities.SetCapability(descriptorNum, simultaneous,extCapability); simultaneous = num; } simultaneous = P_MAX_INDEX; basecapabilities.SetCapability(descriptorNum, simultaneous,new H323ControlExtendedVideoCapability()); delete capability; } } void H323CodecExtendedVideoCapability::AddCapability(const PString & cap) { extCapabilities.Add(H323ExtendedVideoFactory::CreateInstance(cap)); } ------------------------------------------------- I propose to replace the "void H323CodecExtendedVideoCapability::AddCapability(const PString & cap)" to this one: void H323CodecExtendedVideoCapability::AddCapability(const PString & cap) { extCapabilities.Add(H323ExtendedVideoFactory::CreateInstance(cap)->clone()); } Thanks -- Iurii Gordiienko

3 7

H323EndPoint::ParsePartyName issue
by Iurii Gordiienko 22 Jan '16

22 Jan '16

Hi, I'm working with ForwardCall functional and have found a bug if we have forward URL which contains PORT number. For example, for h323:@192.168.1.1:1726 input string H323EndPoint::ParsePartyName will make address string like ip$192.168.1.1:1726:1726. The fix is trivial (may be I've not covered all cases): // get the various parts of the name PString hostOnly = PString(); PINDEX userPos = remoteParty.Find('@'); if (userPos != P_MAX_INDEX) { if (gatekeeper != NULL) alias = url.AsString(); else { alias = remoteParty.Left(userPos); PINDEX portPos = remoteParty.FindLast(':', portPos + 1); PINDEX len = (portPos != P_MAX_INDEX) ? (portPos - userPos - 1) : P_MAX_INDEX; hostOnly = remoteParty.Mid(userPos+1, len); } } else { alias = url.GetUserName(); hostOnly = url.GetHostName(); } address = hostOnly; Also we have similar issue for input string without '@' character. In this case PTLIB::PURL class wrong parse the string (uses 192.168.1.1:1726:1726 as USER). To fix this issue we need look PTLIB::PURL class futher. Thanks -- Iurii Gordiienko

2 1

Twice H323Connection::Unlock() call issue
by Iurii Gordiienko 22 Jan '16

22 Jan '16

Hi, I have found a bug with twice H323Connection::Unlock() call if we have an connection issue. Take a look: 1.We are running connection.SendSignalSetup function from H225CallThread::Main() and if it returns any reason!=H323Connection::EndedByCallerAbort we will Unlock the connection. void H225CallThread::Main() { PTRACE(3, "H225\tStarted call thread"); if (connection.Lock()) { H323Connection::CallEndReason reason = connection.SendSignalSetup(alias, address); // Special case, if we aborted the call then already will be unlocked if (reason != H323Connection::EndedByCallerAbort) connection.Unlock(); // bla-bla-bla } 2.But we have case when connection.SendSignalSetup returns non H323Connection::EndedByCallerAbort reason but has own unlocked state - for this case PTLIB will unlock the mutex twice with assert (as minimum) message. H323Connection::SendSignalSetup(const PString & alias, const H323TransportAddress & address) { //bla-bla-bla // Release the mutex as can deadlock trying to clear call during connect. Unlock(); PBoolean connectFailed = false; if (!signallingChannel->IsOpen()) { signallingChannel->SetWriteTimeout(100); connectFailed = !signallingChannel->Connect(); } // See if transport connect failed, abort if so. if (connectFailed) { connectionState = NoConnectionActive; switch (signallingChannel->GetErrorNumber()) { case ENETUNREACH : return EndedByUnreachable; case ECONNREFUSED : return EndedByNoEndPoint; case ETIMEDOUT : return EndedByHostOffline; } return EndedByConnectFail; } } Thanks -- Iurii Gordiienko

2 3

H323Connection concurrent access issue
by Iurii Gordiienko 22 Jan '16

22 Jan '16

Hi all, I found an issue with concurrent access during work with H323Connection. Take a look on H46018Transport::HandleH46018SignallingChannelPDU(PThread * thread) Simplest schema of this function running: thread1: H46018Transport::HandleH46018SignallingChannelPDU(PThread * thread) { ... H323Connection * connection = endpoint.CreateConnection(callReference, NULL, this, &pdu); ... connection->AttachSignalChannel(token, this, true); ... connection->HandleSignallingChannel(); ... return connection->HadAnsweredCall(); } The function connection->HandleSignallingChannel() in really working with "this" (H46018Transport object) all time in the while-loop. thread2: H323EndPoint::CleanUpConnections() running. In this thread we are running: 1.connection.CleanUpOnCallEnd() - this function will call CleanUpOnTermination() for signallingChannel (H46018Transport object) which running in connection->HandleSignallingChannel() function. 2.delete current H323Connection (which will delete our signallingChannel object also) The concurrent access issue - in time when we are running connection->HandleSignallingChannel() function from thread1, the H323Connection object can be deleted and signallingChannel will points to dead object - we have a crash in result. Also it is possible to run the connection->HadAnsweredCall() function for dead "connection" object. I have had the crash several times for this situation. Thanks -- Iurii Gordiienko

1 0

H323EndPoint::InternalMakeCall bug
by Iurii Gordiienko 03 Jan '16

03 Jan '16

Hi, I have found an issue with H323EndPoint::InternalMakeCall function for case when it running from H323EndPoint::ForwardConnection function (may be for some others cases too). Take a look: 1.We are trying to connect to some host which uses Gatekeeper. 2.From H323EndPoint::ForwardConnection call H323EndPoint::InternalMakeCall function with some internal token="ip$xxx.xxx.xxx.xxx/port" 3.In the body of H323EndPoint::InternalMakeCall we have the replacement current callToken-connection pair to new one with "ip$xxx.xxx.xxx.xxx/port-replaced-1" string, like connectionsActive.SetAt(adjustedToken, connectionsActive.RemoveAt(newToken)); connectionsToBeCleaned += adjustedToken; 4.But if for some reason our virtual CreateConnection function returns NULL... connection = CreateConnection(lastReference, userData, transport, NULL); if (connection == NULL) { PTRACE(1, "H323\tCreateConnection returned NULL"); connectionsMutex.Signal(); return NULL; } ... we cannot clear current Connection because it has not actual callToken "ip$xxx.xxx.xxx.xxx/port" but H323EndPoint::connectionsActive has "ip$xxx.xxx.xxx.xxx/port-replaced-1" and endpoint.ClearCall(callToken, reason) or something similar will be failed because not able to found the Connection by own callToken. 5.Also we have redundant call connectionsMutex.Signal(). if (connection == NULL) { PTRACE(1, "H323\tCreateConnection returned NULL"); connectionsMutex.Signal(); return NULL; } My proposition - just rename back callToken for H323EndPoint::connectionsActive and remove adjustedToken from H323EndPoint::connectionsToBeCleaned if we have NULL connection. My H323EndPoint::InternalMakeCall function looks like this: H323Connection * H323EndPoint::InternalMakeCall(const PString & trasferFromToken, const PString & callIdentity, unsigned capabilityLevel, const PString & remoteParty, H323Transport * transport, PString & newToken, void * userData, PBoolean supplimentary ) { PTRACE(2, "H323\tMaking call to: " << remoteParty); PString alias; H323TransportAddress address; if (!ParsePartyName(remoteParty, alias, address)) { PTRACE(2, "H323\tCould not parse \"" << remoteParty << '"'); return NULL; } #ifdef H323_H46017 // If H.460.17 use the existing H.460.17 Transport if (transport == NULL && RegisteredWithH46017()) transport = GetH46017Transport(); #endif if (transport == NULL) { // Restriction: the call must be made on the same transport as the one // that the gatekeeper is using. if (gatekeeper != NULL) transport = gatekeeper->GetTransport().GetRemoteAddress().CreateTransport(*this); // assume address is an IP address/hostname else transport = address.CreateTransport(*this); if (transport == NULL) { PTRACE(1, "H323\tInvalid transport in \"" << remoteParty << '"'); return NULL; } } H323Connection * connection; connectionsMutex.Wait(); PString adjustedToken; unsigned lastReference; if (newToken.IsEmpty()) { do { lastReference = Q931::GenerateCallReference(); newToken = BuildConnectionToken(*transport, lastReference, FALSE); } while (connectionsActive.Contains(newToken)); } else { lastReference = newToken.Mid(newToken.Find('/')+1).AsUnsigned(); // Move old connection on token to new value and flag for removal unsigned tieBreaker = 0; do { adjustedToken = newToken + "-replaced"; adjustedToken.sprintf("-%u", ++tieBreaker); } while (connectionsActive.Contains(adjustedToken)); connectionsActive.SetAt(adjustedToken, connectionsActive.RemoveAt(newToken)); connectionsToBeCleaned += adjustedToken; PTRACE(3, "H323\tOverwriting call " << newToken << ", renamed to " << adjustedToken); } connectionsMutex.Signal(); connection = CreateConnection(lastReference, userData, transport, NULL); if (connection == NULL) { PTRACE(1, "H323\tCreateConnection returned NULL"); if (!adjustedToken.IsEmpty()) { connectionsMutex.Wait(); connectionsActive.SetAt(newToken, connectionsActive.RemoveAt(adjustedToken)); connectionsToBeCleaned -= adjustedToken; PTRACE(3, "H323\tOverwriting call " << adjustedToken << ", renamed to " << newToken); connectionsMutex.Signal(); } return NULL; } connection->SetRemotePartyName(remoteParty); if (supplimentary) connection->SetNonCallConnection(); connection->Lock(); connectionsMutex.Wait(); connectionsActive.SetAt(newToken, connection); connectionsMutex.Signal(); connection->AttachSignalChannel(newToken, transport, FALSE); #ifdef H323_H450 if (capabilityLevel == UINT_MAX) connection->HandleTransferCall(trasferFromToken, callIdentity); else { connection->HandleIntrudeCall(trasferFromToken, callIdentity); connection->IntrudeCall(capabilityLevel); } #endif PTRACE(3, "H323\tCreated new connection: " << newToken); #ifdef H323_H46017 if (RegisteredWithH46017()) { H323Connection::CallEndReason reason = connection->SendSignalSetup(alias, address); if (reason != H323Connection::NumCallEndReasons) connection->ClearCall(reason); } else #endif new H225CallThread(*this, *connection, *transport, alias, address); return connection; } Thanks -- Iurii Gordiienko

2 1