Summary:	ASTERISK-06548: [patch] incapsulation of ast_best_codec(chan->nativeformats)
Reporter:	Denis Smirnov (mithraen)	Labels:
Date Opened:	2006-03-15 03:07:08.000-0600	Date Closed:	2008-03-17 13:36:09
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/NewFeature
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 6725.2.patch ( 1) 6725.add.functions.patch ( 2) use.inline.functions.patch
Description:	This patch is a part of my work for incapsulate must work with codecs. That needed for future changes with codec negotiation logic. Added functions (all this functions static inline): - ast_chan_best_codec(chan) = ast_best_codec(chan->nativecodecs)   This need, because in future nativecodecs can be changed from int to struct   type; - ast_get_read_format(chan) = chan->readformat - ast_get_write_format(chan) = chan->writeformat - ast_request_inherit -- wrapper around ast_request, that can:    - copy type from parent channel, if type not set;    - copy formats from parent channel; - ast_chan_getformatname  = ast_getformatname(chan->nativeformats) - ast_channel_format_reset =        ast_set_read_format(chan->readformat);        ast_set_write_format(chan->writeformat); First patch adds new functions. Second patch, switch most of Asterisk code to use new functions.
Comments:	By: Olle Johansson (oej) 2006-03-19 11:37:00.000-0600 This code is now implemented in the "team/oej/codecnegotation" branch. By: Denis Smirnov (mithraen) 2006-03-28 12:02:18.000-0600 This patch add ast_get_(read|write)format, ast_chan_best_codec, and patches to all modules where this functions can be used. Patch contains trivial changes, that needed for future codec negotiation changes. By: Denis Smirnov (mithraen) 2006-04-06 04:49:16 team/oej/codecnegotiation updated By: Serge Vecher (serge-v) 2006-05-04 10:13:36 mithraen, another one of your bugs that has disclaimer set to no ... By: Denis Smirnov (mithraen) 2006-05-04 10:32:23 Thanks, Disclaimed. By: Andrey S Pankov (casper) 2006-05-06 11:16:30 Denis: + tmpchan->readformat = ast_get_read_format(chan); + tmpchan->writeformat = ast_get_write_format(chan); Sure here?.. and in a lot of other places? What about consistence of the API in the case? Any possibility to use ast_set_..._format? +    ast_getformatname(ast->nativeformats), what about this and other places as well with ->nativeformats access if we talk about incapsulation? Your patch is a good start for a discussion, but there are still issues with it. It would be nice if you start a thread on the dev@ about the new API :) But I'm afraid this is post-1.4 in any case. By: Denis Smirnov (mithraen) 2006-05-07 12:08:12 1. Patch not _change_ API, only add some new functions. I think it cannot be considered as architecture change, and can be added to trunk. 2. About ast_set -- now this functions do not only set read/writeformats, now I think how change this better. 3. ast_getformatname(ast->nativeformats) -- yes, it must be fixed. I update patch soon  with change to this (adding new ast_chan_getformatname) By: Denis Smirnov (mithraen) 2006-05-20 10:09:31 I upload new patches, based on team/oej/codecnegotiation branch for review. By: Serge Vecher (serge-v) 2006-06-06 13:51:05 Mithraen: since oej doesn't monitor this bug, you may want to send him an email directly... By: Denis Smirnov (mithraen) 2006-06-07 04:21:54 Patches _for_ team/oej/codecnegotiation I send directly to oej. This branch is only copy of my work for API cleanup. OIle help me with keeping my paches up to day in his branch and sometimes test it for compiling. Patches that I post there is only diff between trunk and branch. Patch add.inline.functions.patch ready to go in trunk today, and hasn't side effects. Patch use.inline.functions.patch also ready, but can need to be updated to _today_ trunk when it considered to be merged with trunk. But if it applied with rejects, all that rejects can be silently ignored -- it wouldn't have any side effects. By: Denis Smirnov (mithraen) 2006-06-07 04:22:15 By: Serge Vecher (serge-v) 2006-06-07 11:35:13 Mithraen: ACK By: Andrey S Pankov (casper) 2006-06-07 14:53:11 vechers: How did add.inline.functions.patch pass formatting review with if( type == NULL ) chan? chan->nativeformats: 0 if( r ) if( w ) etc... strings in it? By: Serge Vecher (serge-v) 2006-06-07 14:57:26 hmm ... looks like I need to get glasses here; good catch, thanks. By: Andrey S Pankov (casper) 2006-06-07 15:13:38 > Patch add.inline.functions.patch ready to go in trunk today, and hasn't side effects. Mithraen: are you sure about     return ast_getformatname(chan? chan->nativeformats: 0); ? What the sense of calling ast_getformatname with 0? Why not to check whether option_debug is set when ast_log'ging to LOG_DEBUG? Why do you use both ast_chan_ and ast_channel_ as prefixes? What about being consistent here? Currently there are no API calls with ast_chan_. By: Denis Smirnov (mithraen) 2006-06-07 17:24:31 Thanks, Casper. I fix this issues By: Denis Smirnov (mithraen) 2006-06-07 17:28:27 Patch updated By: Denis Smirnov (mithraen) 2006-07-01 01:23:58 Why after 1.4? At least API adding part can be integrated anytime. By: Serge Vecher (serge-v) 2006-07-05 09:15:00 Mithraen: I marked this as post 1.4 as per development team on #asterisk-dev channel. If kpfleming has committed to merging this in prior to 1.4 beta release, please send him a reminder directly. By: Denis Smirnov (mithraen) 2006-08-21 04:28:48 Updated to last trunk By: Denis Smirnov (mithraen) 2006-09-21 16:31:10 Would 6725.add.functions.patch be appied to trunk now? By: Denis Smirnov (mithraen) 2006-09-21 16:32:13 By: jmls (jmls) 2006-11-01 12:45:19.000-0600 it would be appreciated if you could have a look at this. Thanks By: Denis Smirnov (mithraen) 2007-01-10 13:52:30.000-0600 What about this patch? When it can be integrated? I can update it to last trunk if needed. By: Serge Vecher (serge-v) 2007-01-10 15:14:15.000-0600 mithraen: the patch is pretty dated, I think it is best to update it... Also, what the deal with the bug_4825 ifdef in the code? By: Denis Smirnov (mithraen) 2007-01-11 05:41:54.000-0600 ifdef bug_4835 was done for more easy integrate patch from 4825. I can remove this from main patch, and support in separate branch. By: Denis Smirnov (mithraen) 2007-01-11 06:06:52.000-0600 Patch updated. By: Leif Madsen (lmadsen) 2007-10-31 07:53:08 This patch has been ready for testing for a while now. What needs to be tested on it, and what can I do to help move it forward? By: Tilghman Lesher (tilghman) 2008-03-14 12:15:23 I'm currently working on expanding the number of codec bits, and I don't see these patches as compatible with those efforts. By: Joshua C. Colp (jcolp) 2008-03-17 13:35:29 Per Corydon, this is going to be taken care of in his new codec bits branch and no longer applicable.