mirror of
https://github.com/asterisk/asterisk.git
synced 2025-10-08 06:12:45 +00:00
6bb45831eb
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
94 lines
3.5 KiB
C
94 lines
3.5 KiB
C
/*
|
|
* Prototypes for public functions only of internal interest,
|
|
* normally not used by modules.
|
|
* What goes here are typically *_init() routines.
|
|
*/
|
|
|
|
/*! \file
|
|
*
|
|
* \brief
|
|
* Prototypes for public functions only of internal interest,
|
|
*
|
|
*/
|
|
|
|
|
|
#ifndef _ASTERISK__PRIVATE_H
|
|
#define _ASTERISK__PRIVATE_H
|
|
|
|
int load_modules(unsigned int); /*!< Provided by loader.c */
|
|
int load_pbx(void); /*!< Provided by pbx.c */
|
|
int init_logger(void); /*!< Provided by logger.c */
|
|
void close_logger(void); /*!< Provided by logger.c */
|
|
int init_framer(void); /*!< Provided by frame.c */
|
|
int ast_term_init(void); /*!< Provided by term.c */
|
|
int astdb_init(void); /*!< Provided by db.c */
|
|
void ast_channels_init(void); /*!< Provided by channel.c */
|
|
void ast_builtins_init(void); /*!< Provided by cli.c */
|
|
int ast_cli_perms_init(int reload); /*!< Provided by cli.c */
|
|
int dnsmgr_init(void); /*!< Provided by dnsmgr.c */
|
|
void dnsmgr_start_refresh(void); /*!< Provided by dnsmgr.c */
|
|
int dnsmgr_reload(void); /*!< Provided by dnsmgr.c */
|
|
void threadstorage_init(void); /*!< Provided by threadstorage.c */
|
|
int ast_event_init(void); /*!< Provided by event.c */
|
|
int ast_device_state_engine_init(void); /*!< Provided by devicestate.c */
|
|
int astobj2_init(void); /*!< Provided by astobj2.c */
|
|
int ast_file_init(void); /*!< Provided by file.c */
|
|
int ast_features_init(void); /*!< Provided by features.c */
|
|
void ast_autoservice_init(void); /*!< Provided by autoservice.c */
|
|
int ast_data_init(void); /*!< Provided by data.c */
|
|
int ast_http_init(void); /*!< Provided by http.c */
|
|
int ast_http_reload(void); /*!< Provided by http.c */
|
|
int ast_tps_init(void); /*!< Provided by taskprocessor.c */
|
|
int ast_timing_init(void); /*!< Provided by timing.c */
|
|
int ast_indications_init(void); /*!< Provided by indications.c */
|
|
int ast_indications_reload(void);/*!< Provided by indications.c */
|
|
void ast_stun_init(void); /*!< Provided by stun.c */
|
|
int ast_cel_engine_init(void); /*!< Provided by cel.c */
|
|
int ast_cel_engine_reload(void); /*!< Provided by cel.c */
|
|
int ast_ssl_init(void); /*!< Provided by ssl.c */
|
|
int ast_test_init(void); /*!< Provided by test.c */
|
|
|
|
/*!
|
|
* \brief Reload asterisk modules.
|
|
* \param name the name of the module to reload
|
|
*
|
|
* This function reloads the specified module, or if no modules are specified,
|
|
* it will reload all loaded modules.
|
|
*
|
|
* \note Modules are reloaded using their reload() functions, not unloading
|
|
* them and loading them again.
|
|
*
|
|
* \return 0 if the specified module was not found.
|
|
* \retval 1 if the module was found but cannot be reloaded.
|
|
* \retval -1 if a reload operation is already in progress.
|
|
* \retval 2 if the specfied module was found and reloaded.
|
|
*/
|
|
int ast_module_reload(const char *name);
|
|
|
|
/*!
|
|
* \brief Process reload requests received during startup.
|
|
*
|
|
* This function requests that the loader execute the pending reload requests
|
|
* that were queued during server startup.
|
|
*
|
|
* \note This function will do nothing if the server has not completely started
|
|
* up. Once called, the reload queue is emptied, and further invocations
|
|
* will have no affect.
|
|
*/
|
|
void ast_process_pending_reloads(void);
|
|
|
|
/*! \brief Load XML documentation. Provided by xmldoc.c
|
|
* \retval 1 on error.
|
|
* \retval 0 on success.
|
|
*/
|
|
int ast_xmldoc_load_documentation(void);
|
|
|
|
/*!
|
|
* \brief Reload genericplc configuration value from codecs.conf
|
|
*
|
|
* Implementation is in main/channel.c
|
|
*/
|
|
int ast_plc_reload(void);
|
|
|
|
#endif /* _ASTERISK__PRIVATE_H */
|