Skip to content
Snippets Groups Projects
  1. May 26, 2010
  2. May 25, 2010
  3. May 24, 2010
  4. May 23, 2010
  5. May 21, 2010
  6. May 20, 2010
  7. May 19, 2010
    • Mark Michelson's avatar
      Fix transcode_via_sln option with SIP calls and improve PLC usage. · 6bb45831
      Mark Michelson authored
      From reviewboard:
      The problem here is a bit complex, so try to bear with me...
      
      It was noticed by a Digium customer that generic PLC (as configured in
      codecs.conf) did not appear to actually be having any sort of benefit when
      packet loss was introduced on an RTP stream. I reproduced this issue myself
      by streaming a file across an RTP stream and dropping approx. 5% of the
      RTP packets. I saw no real difference between when PLC was enabled or disabled
      when using wireshark to analyze the RTP streams.
      
      After analyzing what was going on, it became clear that one of the problems
      faced was that when running my tests, the translation paths were being set
      up in such a way that PLC could not possibly work as expected. To illustrate,
      if packets are lost on channel A's read stream, then we expect that PLC will
      be applied to channel B's write stream. The problem is that generic PLC can
      only be done when there is a translation path that moves from some codec to
      SLINEAR. When I would run my tests, I found that every single time, read
      and write translation paths would be set up on channel A instead of channel
      B. There appeared to be no real way to predict which channel the translation
      paths would be set up on.
      
      This is where Kevin swooped in to let me know about the transcode_via_sln
      option in asterisk.conf. It is supposed to work by placing a read translation
      path on both channels from the channel's rawreadformat to SLINEAR. It also
      will place a write translation path on both channels from SLINEAR to the
      channel's rawwriteformat. Using this option allows one to predictably set up
      translation paths on all channels. There are two problems with this, though.
      First and foremost, the transcode_via_sln option did not appear to be working
      properly when I was placing a SIP call between two endpoints which did not
      share any common formats. Second, even if this option were to work, for PLC
      to be applied, there had to be a write translation path that would go from
      some format to SLINEAR. It would not work properly if the starting format
      of translation was SLINEAR.
      
      The one-line change presented in this review request in chan_sip.c fixed the
      first issue for me. The problem was that in sip_request_call, the
      jointcapability of the outbound channel was being set to the format passed to
      sip_request_call. This is nativeformats of the inbound channel. Because of this,
      when ast_channel_make_compatible was called by app_dial, both channels already
      had compatibly read and write formats. Thus, no translation path was set up at
      the time. My change is to set the jointcapability of the sip_pvt created during
      sip_request_call to the intersection of the inbound channel's nativeformats and
      the configured peer capability that we determined during the earlier call to
      create_addr. Doing this got the translation paths set up as expected when using
      transcode_via_sln.
      
      The changes presented in channel.c fixed the second issue for me. First and
      foremost, when Asterisk is started, we'll read codecs.conf to see the value of
      the genericplc option. If this option is set, and ast_write is called for a
      frame with no data, then we will attempt to fill in the missing samples for
      the frame. The implementation uses a channel datastore for maintaining the
      PLC state and for creating a buffer to store PLC samples in. Even when we
      receive a frame with data, we'll call plc_rx so that the PLC state will have
      knowledge of the previous voice frame, which it can use as a basis for when
      it comes time to actually do a PLC fill-in.
      
      So, reviewers, now I ask for your help. First off, there's the one line change
      in chan_sip that I have put in. Is it right? By my logic it seems correct, but
      I'm sure someone can tell me why it is not going to work. This is probably the
      change I'm least concerned about, though. What concerns me much more is the
      set of changes in channel.c. First off, am I even doing it right? When I run
      tests, I can clearly see that when PLC is activated, I see a significant increase
      in RTP traffic where I would expect it to be. However, in my humble opinion, the
      audio sounds kind of crappy whenever the PLC fill-in is done. It sounds worse to
      me than when no PLC is used at all. I need someone to review the logic I have used
      to be sure that I'm not misusing anything. As far as I can see my pointer arithmetic
      is correct, and my use of AST_FRIENDLY_OFFSET should be correct as well, but I'm
      sure someone can point out somewhere where I've done something incorrectly.
      
      As I was writing this review request up, I decided to give the code a test run under
      valgrind, and I find that for some reason, calls to plc_rx are causing some invalid
      reads. Apparently I'm reading past the end of a buffer somehow. I'll have to dig around
      a bit to see why that is the case. If it's obvious to someone reviewing, speak up!
      
      Finally, I have one other proposal that is not reflected in my code review. Since
      without transcode_via_sln set, one cannot predict or control where a translation
      path will be up, it seems to me that the current practice of using PLC only when
      transcoding to SLINEAR is not useful. I recommend that once it has been determined
      that the method used in this code review is correct and works as expected, then
      the code in translate.c that invokes PLC should be removed.
      
      Review: https://reviewboard.asterisk.org/r/622/
      
      
      
      git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@264452 65c4cc65-6c06-0410-ace0-fbb531ad65f3
      6bb45831
    • David Vossel's avatar
      fixes infinite loop during udptl.c's decode_open_type · d7e9d071
      David Vossel authored
      When decode_length returns the length there is a check to see if that
      length is negative, if so the decode loop breaks as this means the
      limit has been reached.  The problem here is that length is an
      unsigned int, so length can never be negative.  This resulted in
      an infinite loop.
      
      (issue #17352)
      
      
      
      git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@264400 65c4cc65-6c06-0410-ace0-fbb531ad65f3
      d7e9d071
    • Matthew Nicholson's avatar
      Cast an unsigned int to a signed int when comparing it with 0. · 6eaf9b87
      Matthew Nicholson authored
      (AST-377)
      
      
      git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@264379 65c4cc65-6c06-0410-ace0-fbb531ad65f3
      6eaf9b87
    • Matthew Nicholson's avatar
      Merged revisions 264334 via svnmerge from · d38c6459
      Matthew Nicholson authored
      https://origsvn.digium.com/svn/asterisk/branches/1.4
      
      ........
        r264334 | mnicholson | 2010-05-19 15:01:38 -0500 (Wed, 19 May 2010) | 5 lines
        
        Set quieted flag when receiving a dtmf tone during playback in speechbackground.
        
        (closes issue #16966)
        Reported by: asackheim
      ........
      
      
      git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@264335 65c4cc65-6c06-0410-ace0-fbb531ad65f3
      d38c6459
    • David Vossel's avatar
      fixes crash in check_rtp_timeout · 0407208b
      David Vossel authored
      During deadlock avoidance the sip dialog pvt is locked and
      unlocked.  When this occurs we have no guarantee the pvt's owner
      is still valid.  We were trying to access the pvt's owner after
      this without checking to see if it still existed first. 
      
      (closes issue #17271)
      Reported by: under
      Patches:
            check_rtp_timeout.diff uploaded by under (license 914)
      Tested by: dvossel
      
      
      git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@264331 65c4cc65-6c06-0410-ace0-fbb531ad65f3
      0407208b
    • Tilghman Lesher's avatar
      Merged revisions 264248 via svnmerge from · b5a62962
      Tilghman Lesher authored
      https://origsvn.digium.com/svn/asterisk/branches/1.4
      
      ........
        r264248 | tilghman | 2010-05-19 12:41:29 -0500 (Wed, 19 May 2010) | 17 lines
        
        Internal timing is now on by default, if you're using DAHDI 2.3 or above.
        
        The reason for ensuring DAHDI 2.3 or above is that this version ensures that
        a timer is always available, whereas in previous versions, it was possible
        for DAHDI to be loaded, but have no drivers to actually generate timing.  If
        internal_timing was turned on in this circumstance, a complete lack of audio
        would result.  This is the reason why internal_timing was not on by default.
        However, now that DAHDI ensures the availability of a timer, there is no
        reason for this setting to be off (and in fact, it solves a great many initial
        user problems).
        
        (closes issue #15932)
         Reported by: dimas
         Patches: 
               20100519__issue15932.diff.txt uploaded by tilghman (license 14)
         Tested by: tilghman
      ........
      
      
      git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@264249 65c4cc65-6c06-0410-ace0-fbb531ad65f3
      b5a62962
    • Tilghman Lesher's avatar
      Keep track of digit duration, when we're decoding inband to pass DTMF frames. · 07df131a
      Tilghman Lesher authored
      (closes issue #17235)
       Reported by: frawd
       Patches: 
             new_dtmf_dsp_len.patch uploaded by frawd (license 610)
             20100518__issue17235.diff.txt uploaded by tilghman (license 14)
       Tested by: frawd
      
      
      git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@264204 65c4cc65-6c06-0410-ace0-fbb531ad65f3
      07df131a
Loading