Hi Raphael,

unfortunately, the code you checked in does not work for static
payload types. The attached patch fixes the problem.

Regards,
Emil

Am Thu, 16 Jun 2011 14:14:30 +0200
schrieb Raphael Coeffic <[email protected]>:

> 
> Thanks a lot Emil! The patch has been commited to master (cd6efde).
> 
> -Raphael.
> 
> On 16.06.11 12:53, Emil Kroymann wrote:
> > OK,
> >
> > apparently there is a second mapping from remote payload type to
> > codec already previewed in current SEMS master. However, because of
> > two minor bugs, this mapping wasn't used. Attached is a patch that
> > fixes these bugs. With this patch applied SEMS sends RTP with
> > payload types according to the mapping given in the remote SDP.
> >
> > Regarding the "SHOULD" in the RFC reported by Matthew: This can be
> > fixed by introducing a second "local" mapping from payload type to
> > codec per RTP stream. However, I currently have no time to
> > implement such a thing, since the behavior without the should is
> > acceptable for us at the moment.
> >
> > Regards,
> > Emil
> >
> > Am Mon, 13 Jun 2011 14:44:54 +0200
> > schrieb Emil Kroymann<[email protected]>:
> >
> >> Hi Raphael,
> >>
> >> IMHO we need two mappings from payload type two codec per RTP
> >> session anyway, since when SEMS is generating the offer, the
> >> remote end can answer with any mapping it likes and SEMS must use
> >> this mapping for sending. If the payload type mapping in SEMS is
> >> not global anymore, we can adjust the mapping in the SDP answer
> >> generated by SEMS to whatever is given in the offer.
> >>
> >> Regards,
> >> Emil
> >>
> >> Am Sun, 12 Jun 2011 08:39:31 +0200
> >> schrieb Raphael Coeffic<[email protected]>:
> >>
> >>> Hi Emil, Matthew,
> >>>
> >>> I'm quite sure how to realize such a "remote bugfix", as it could
> >>> overlap with locally define codecs...
> >>> By the way, I've seen this behavior with Jitsi (aka. SIP
> >>> communicator). However, my snom 360 behaves correctly.
> >>> Neither of the other UAs I have ever used do expose such a
> >>> missbehavior.
> >>>
> >>> Matthew: were you using Jitsi (possibly the osx versiob ;-))? If
> >>> yes, we should definitely file a bug report.
> >>>
> >>> Cheers
> >>> Raphael.
> >>>
> >>> On 11.06.11 17:53, Emil Kroymann wrote:
> >>>> Hi Matthew,
> >>>>
> >>>> I think in this case it is the peer behaving incorrectly. The
> >>>> peer should send under the payload types offered by SEMS and SEMS
> >>>> vice-versa under the payload types given in the answer.
> >>>>
> >>>> Still it seems to be desirable for interoperability, if SEMS
> >>>> would match the payload types given by the peer. In the case
> >>>> where SEMS is generating the SDP offer, this is not possible,
> >>>> however.
> >>>>
> >>>> Regards,
> >>>> Emil
> >>>>
> >>>> Am Sat, 11 Jun 2011 11:02:10 -0400
> >>>> schrieb Matthew Williams<[email protected]>:
> >>>>
> >>>>> Hi,
> >>>>>
> >>>>> I think I'm finding a very similar issue with payload types for
> >>>>> telephone-events. If this is unrelated, please accept my
> >>>>> apologies.
> >>>>>
> >>>>> A peer is sending an INVITE with a payload type of '101' for
> >>>>> telephone-events. In SEMS' reply (200 OK), payload type '96' is
> >>>>> indicated for telephone-events. When the peer sends an event
> >>>>> with type 101, SEMS rejects it as unknown type. I was actually
> >>>>> expecting SEMS to reply with the same payload type (101), but
> >>>>> from what you're saying, it seems the sdp itself is acceptable,
> >>>>> but the fact that SEMS is rejecting payload type 101 is not? Or
> >>>>> is it the peer that should be sending as type 96, since this is
> >>>>> what was in the SDP reply?
> >>>>>
> >>>>> I am working with latest master
> >>>>> (856a7428eaea24a426688b277b2d504b4cb1f798).
> >>>>>
> >>>>> Regards,
> >>>>> Matthew Williams
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> On Sat, Jun 11, 2011 at 7:05 AM, Emil Kroymann
> >>>>> <[email protected]>wrote:
> >>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> I think I found a problem with the SDP offer-answer
> >>>>>> implementation recently merged into master while implementing
> >>>>>> wideband functionality. The problem is with the assignement of
> >>>>>> dynamic payload types. SEMS seems to keep only one mapping of
> >>>>>> dynamic payload types to codec implementation, which is global
> >>>>>> to all of SEMS. However, it should keep two separate mappings -
> >>>>>> one for sending and one for receiving - for each RTP stream.
> >>>>>> This problem shows in the following setup:
> >>>>>>
> >>>>>> I use twinkle to setup a call to SEMS, which plays an
> >>>>>> announcement using the speex wideband codec. Twinkle sends the
> >>>>>> following SDP in the INVITE:
> >>>>>>
> >>>>>> v=0.
> >>>>>> o=twinkle 1229564934 1632772146 IN IP4 192.168.1.121.
> >>>>>> s=-.
> >>>>>> c=IN IP4 XX.XX.XX.XX.
> >>>>>> t=0 0.
> >>>>>> m=audio 13006 RTP/AVP 98 97 8 0 102 3 101.
> >>>>>> a=rtpmap:98 speex/16000.
> >>>>>> a=rtpmap:97 speex/8000.
> >>>>>> a=rtpmap:8 PCMA/8000.
> >>>>>> a=rtpmap:0 PCMU/8000.
> >>>>>> a=rtpmap:102 G726-16/8000.
> >>>>>> a=rtpmap:3 GSM/8000.
> >>>>>> a=rtpmap:101 telephone-event/8000.
> >>>>>> a=fmtp:101 0-15.
> >>>>>> a=ptime:20.
> >>>>>> a=nortpproxy:yes.
> >>>>>>
> >>>>>> This means, that twinkle expects the speex wideband codec under
> >>>>>> payload type 98 and the speex narrowband codec under payload
> >>>>>> type 97.
> >>>>>>
> >>>>>> SEMS answers with the following SDP in the 200 OK:
> >>>>>>
> >>>>>> v=0. o=sems 1 1 IN IP4 XX.XX.XX.XX.
> >>>>>> s=sems.
> >>>>>> c=IN IP4 89.246.236.49.
> >>>>>> t=0 0.
> >>>>>> m=audio 10000 RTP/AVP 97 96 8 0 101.
> >>>>>> a=rtpmap:97 speex/16000.
> >>>>>> a=rtpmap:96 speex/8000.
> >>>>>> a=rtpmap:8 PCMA/8000.
> >>>>>> a=rtpmap:0 PCMU/8000.
> >>>>>> a=rtpmap:101 telephone-event/8000.
> >>>>>>
> >>>>>> This means, that SEMS expects the speex wideband codec under
> >>>>>> payload type 97 and the speex narrowband codec under payload
> >>>>>> type 96.
> >>>>>>
> >>>>>> Unfortunately, SEMS sends speex wideband encoded RTP packets
> >>>>>> with payload type 97. This leads to twinkle decoding the audio
> >>>>>> with the narrowband decoder and to poor audio quality.
> >>>>>>
> >>>>>> As mentioned above, to fix the problem, SEMS has to keep a
> >>>>>> seperate payload type mapping for sending and it has to set the
> >>>>>> payload type on outgoing packets according to this mapping.
> >>>>>>
> >>>>>> Since we need this functionality at ISACO quite urgently, I
> >>>>>> would be willing to implement a fix for this.
> >>>>>>
> >>>>>> What do you think?
> >>>>>>
> >>>>>> Regards,
> >>>>>> Emil
> >>>>>>
> >>>>>> --
> >>>>>> Emil Kroymann
> >>>>>> VoIP Services Engineer
> >>>>>>
> >>>>>> Email: [email protected]
> >>>>>> Tel: +49-30-203899885
> >>>>>> Mobile: +49-176-38389303
> >>>>>>
> >>>>>> ISACO GmbH
> >>>>>> Kurfürstenstraße 79
> >>>>>> 10787 Berlin
> >>>>>> Germany
> >>>>>>
> >>>>>> Amtsgericht Charlottenburg, HRB 112464B
> >>>>>> Geschäftsführer: Daniel Frommherz
> >>>>>>
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> Sems mailing list
> >>>>>> [email protected]
> >>>>>> http://lists.iptel.org/mailman/listinfo/sems
> >>>>>>
> >>>>>>
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> Sems mailing list
> >>>> [email protected]
> >>>> http://lists.iptel.org/mailman/listinfo/sems
> >>
> >>
> >
> >
> >
> > _______________________________________________
> > Sems mailing list
> > [email protected]
> > http://lists.iptel.org/mailman/listinfo/sems
> 



-- 
Emil Kroymann
VoIP Services Engineer

Email: [email protected]
Tel: +49-30-203899885
Mobile: +49-176-38389303

ISACO GmbH
Kurfürstenstraße 79
10787 Berlin
Germany

Amtsgericht Charlottenburg, HRB 112464B
Geschäftsführer: Daniel Frommherz

--- a/core/AmRtpStream.cpp
+++ b/core/AmRtpStream.cpp
@@ -326,11 +326,11 @@
     return -1;
 
   PayloadMappingTable::iterator it = pl_map.find(payload);
-  if ((it == pl_map.end()) || (it->second.remote_pt < 0)) {
+  if ((it == pl_map.end())) {
     return -1;
   }
   
-  return compile_and_send(it->second.remote_pt, false, ts, buffer, size);
+  return compile_and_send(it->second.remote_pt < 0 ? payload : it->second.remote_pt, false, ts, buffer, size);
 }
 
 int AmRtpStream::send_raw( char* packet, unsigned int length )

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Sems mailing list
[email protected]
http://lists.iptel.org/mailman/listinfo/sems

Reply via email to