Chris, Upon closer inspection, there is a flaw in that refactored min/max macro. The bug is exposed when the value is higher than the maximum value. The order of evaluation eventually gets down to returning the original value, rather than the maximum value. Here's the comparison order: 1. if value < maxvalue -> equates false, so maxvalue is returned 2. if minvalue < maxvalue -> equates true, so value is returned (!!) That's the problem. So, I have reverted to a simpler implementation, which is actually my original implementation. It works, and is easily understandable, and in this instance does not have any side-effects. Here is my macro definition that I added to math.h: #define PJ_MIN_MAX(_value, _minval, _maxval) PJ_MAX(_minval, PJ_MIN(_maxval, _value)) This makes it easy to understand the intent. If this were C++ code, it would be easier to use templates without fear of side-effects. Oh well. At least implemented this way, it allows easy replacement of the underlying PJ_MAX and PJ_MIN macros with some other kind of implementation. Regards, Jim Gomes From: pjsip-bounces@xxxxxxxxxxxxxxx [mailto:pjsip-bounces at lists.pjsip.org] On Behalf Of Jim Gomes Sent: Wednesday, 11 March, 2009 8:33 AM To: pjsip list Subject: Re: quick question about volume and PJSUA-LIB As I integrated this into the source code, I found a better name and a good place for it. I placed it in the math.h file located in pjlib\include\pj folder. I also renamed it to be PJ_MIN_MAX and placed it next to the existing PJ_MIN and PJ_MAX macros. Regards, Jim Gomes From: pjsip-bounces@xxxxxxxxxxxxxxx [mailto:pjsip-bounces at lists.pjsip.org] On Behalf Of Jim Gomes Sent: Wednesday, 11 March, 2009 8:26 AM To: pjsip list Subject: Re: quick question about volume and PJSUA-LIB Hi Chris, Glad to hear that you were able to get things working. I also went with expanding the macro inline. Based on your modified macro expansion (from your other follow-up message to correct a less-than symbol), I am adding the following macro to my utility headers: #define min_max(_value, _minval, _maxval) \ ((_minval) < ((_value) < (_maxval) ? (_value) : (_maxval)) ? (_valuel) : (_minval)) Using this macro, I can then change the line for newlevel assignment as follows: int newlevel = min_max((int) level, 0, 255); Thanks for sharing your results. This modified implementation made it much easier for me to map the volume level to a standard slider control. Regards, Jim Gomes From: pjsip-bounces@xxxxxxxxxxxxxxx [mailto:pjsip-bounces at lists.pjsip.org] On Behalf Of C R McClenaghan Sent: Tuesday, 10 March, 2009 8:13 AM To: pjsip list Subject: Re: quick question about volume and PJSUA-LIB Jim, Thanks for the detailed explanation/work around. I did expand the macros in a hand coded fashion: int newlevel = (0 > ((int) level < 255 ? (int) level : 255) ? (int) level : 0); I'm building in a Mac OS X Leopard environment, and while the file would compile (i.e., the header definition for the macro was found), the _max and _min came up undefined by the linker. The manual expansion was much easier than figuring out why the linker failed. Chris On Mar 9, 2009, at 10:54 AM, Jim Gomes wrote: Hi Chris, The min/max are macros from the stdlib.h or from minmax.h file. If you are getting compile errors, you can try renaming them to the ANSI compliant names __max and __min. You can either add #include <stdlib.h> or add #include <minmax.h> at the top of the file, or here are the macro definitions: #ifndef __cplusplus #define max(a,b) (((a) > (b)) ? (a) : (b)) #define min(a,b) (((a) < (b)) ? (a) : (b)) #endif /* __cplusplus */ You can add that to the top of the file as well. Or, you can expand the macros out and hand code them inline like so: int newlevel = (255 < (int) level) ? 255 : (int) level; newlevel = (0 > newlevel) ? 0 : newlevel; I may change my implementation to the hand-coded expansion, since C macro expansion can be somewhat inefficient. Or I may create some special inline functions that would allow me to use the original implementation without the macro expansion side-effects. If you are compiling as C++, you can add #include <xutility> at the top of the file and use the _cpp_min()/_cpp_max() template inline functions that have no macro expansion side-effects. These inline template functions are also aliased to the _MIN and _MAX macros. Refer to the xutility file in your compiler directories for more info. Regards, Jim Gomes From: pjsip-bounces@xxxxxxxxxxxxxxx<mailto:pjsip-bounces at lists.pjsip.org> [mailto:pjsip-bounces at lists.pjsip.org] On Behalf Of C R McClenaghan Sent: Friday, 06 March, 2009 5:35 PM To: pjsip list Subject: Re: quick question about volume and PJSUA-LIB Hey Jim, Thanks, I may need this. I was able to modify the pjsua sample application to accept first a conference port number and then a volume adjust. It had previously been hardcoded to port 0 - but, hey, its a sample program. Anyway it now defaults to 0 but you can specify other ports and a listing of active ports is provided ala the connect/disconnect dialog. I'm going to insert your update and see how I like it. Can you tell which library contains min and max, I'm getting a load error on the build. Thanks, Chris On Mar 6, 2009, at 4:52 PM, Jim Gomes wrote: Hi Chris, I was doing some work in this area last week for one of my own projects. I had great difficulty with this part of the library as well. I eventually patched it to the following to get it to work. /* * Adjust the signal level to be transmitted from the bridge to the * specified port by making it louder or quieter. */ PJ_DEF(pj_status_t) pjsua_conf_adjust_tx_level(pjsua_conf_port_id slot, float level) { int newlevel = max(0, min(255, (int) level)); return pjmedia_conf_adjust_tx_level(pjsua_var.mconf, slot, newlevel - 128); } /* * Adjust the signal level to be received from the specified port (to * the bridge) by making it louder or quieter. */ PJ_DEF(pj_status_t) pjsua_conf_adjust_rx_level(pjsua_conf_port_id slot, float level) { int newlevel = max(0, min(255, (int) level)); return pjmedia_conf_adjust_rx_level(pjsua_var.mconf, slot, newlevel - 128); } With this implementation, the level parameter has a valid range of 0 to 255, with 0 being mute. Feel free to patch your version of the code, because the existing implementation is broken. Regards, Jim Gomes From: pjsip-bounces@xxxxxxxxxxxxxxx<mailto:pjsip-bounces at lists.pjsip.org> [mailto:pjsip-bounces at lists.pjsip.org] On Behalf Of C R McClenaghan Sent: Friday, 06 March, 2009 2:42 PM To: pjsip list Subject: quick question about volume and PJSUA-LIB All, Here's the documentation from online: pj_status_t<http://www.pjsip.org/pjlib/docs/html/group__PJ__BASIC.htm#gb43ba3167bd2a2ab4580509dbf79200e> pjsua_conf_adjust_rx_level ( pjsua_conf_port_id<http://www.pjsip.org/pjsip/docs/html/group__PJSUA__LIB__BASE.htm#gf5d44947e4e62dc31dfde88884534385> slot, float level ) Adjust the signal level to be received from the specified port (to the bridge) by making it louder or quieter. Parameters: slot The conference bridge slot number. level Signal level adjustment. Value 1.0 means no level adjustment, while value 0 means to mute the port. Returns: PJ_SUCCESS on success, or the appropriate error code. So, what are the appropriate ranges for the value of level? Are they 0 to 1? Are the values absolutes or relative to current volume? I'm playing with pjsua application and not sure I can tell. I'd like to see how fine grain the control can be. Thanks, Chris _______________________________________________ Visit our blog: http://blog.pjsip.org pjsip mailing list pjsip at lists.pjsip.org<mailto:pjsip at lists.pjsip.org> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org _______________________________________________ Visit our blog: http://blog.pjsip.org pjsip mailing list pjsip at lists.pjsip.org<mailto:pjsip at lists.pjsip.org> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.pjsip.org/pipermail/pjsip_lists.pjsip.org/attachments/20090311/c2797f51/attachment-0001.html>
