[asterisk-dev] [Code Review] New Media Architecture phase 1 step 1

[David Vossel]

> On 2010-12-20 14:02:14, Russell Bryant wrote:
> > /trunk/main/format_cap.c, lines 64-78
> > <https://reviewboard.asterisk.org/r/1062/diff/2/?file=14761#file14761line64>
> >
> >     Any reason not to go ahead and make this an astobj2 object so ref counts are available if needed?

I don't expect ref counts will be necessary for this object since the functionality it is replacing never required ref counting.  If they are needed in the future, it will be trivial to change this allocation.


> On 2010-12-20 14:02:14, Russell Bryant wrote:
> > /trunk/main/format.c, lines 258-263
> > <https://reviewboard.asterisk.org/r/1062/diff/2/?file=14760#file14760line258>
> >
> >     What about just checking for an existing interface and returning and logging an error if one is found?  With it written this way a module could cause an interface registered by another module to get unregistered silently.  That seems odd to me.

You're right, this is odd.


> On 2010-12-20 14:02:14, Russell Bryant wrote:
> > /trunk/include/asterisk/format.h, lines 212-215
> > <https://reviewboard.asterisk.org/r/1062/diff/2/?file=14758#file14758line212>
> >
> >     It might be worth explicitly documenting that this is a shallow copy of ast_format.

I know what you are hinting at here, but it is not currently allowed for the format attribute field to point to dynamically allocated memory.  So, right now this is a deep copy.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1062/#review3054
-----------------------------------------------------------


On 2010-12-20 12:48:11, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1062/
> -----------------------------------------------------------
> 
> (Updated 2010-12-20 12:48:11)
> 
> 
> Review request for Asterisk Developers and Russell Bryant.
> 
> 
> Summary
> -------
> 
> Here is what I am working on. https://wiki.asterisk.org/wiki/display/AST/Media+Architecture+Proposal
> 
> This is phase 1 step 1 of that proposal.
> 
> Below is everything that is included in this review.
> 
> Step 1
>     * Define new format unique ID system using numbers rather than bits. Allow this definition to remain unused during this step except by the new APIs.
>     * Create Ast Format API + unit tests.
>     * Create Ast Capibility API + unit tests.
>     * Create IAX2 Conversion layer for ast_format and ast_cap objects. Create unit tests and leave this layer inactive until conversion to new APIs takes place.
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/format.h PRE-CREATION 
>   /trunk/include/asterisk/format_cap.h PRE-CREATION 
>   /trunk/main/format.c PRE-CREATION 
>   /trunk/main/format_cap.c PRE-CREATION 
>   /trunk/tests/test_format_api.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/1062/diff
> 
> 
> Testing
> -------
> 
> Unit tests are included.
> 
> 
> Thanks,
> 
> David
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20101220/f0a9b445/attachment-0001.htm>