[FFmpeg-cvslog] r22119 - in trunk/libavutil: tree.c tree.h

[Michael Niedermayer]

On Tue, Mar 02, 2010 at 07:18:09PM +0100, Vitor Sessak wrote:
> Michael Niedermayer wrote:
>> On Tue, Mar 02, 2010 at 06:40:03AM +0100, Vitor Sessak wrote:
>>> Michael Niedermayer wrote:
>>>> On Sun, Feb 28, 2010 at 09:48:42PM +0100, vitor wrote:
>>>>> Author: vitor
>>>>> Date: Sun Feb 28 21:48:42 2010
>>>>> New Revision: 22119
>>>>>
>>>>> Log:
>>>>> Implement av_tree_destroy_free_elem() to destroy a tree and free all 
>>>>> the values stored on it.
>>>>>
>>>>> Modified:
>>>>>    trunk/libavutil/tree.c
>>>>>    trunk/libavutil/tree.h
>>>>>
>>>>> Modified: trunk/libavutil/tree.c
>>>>> ==============================================================================
>>>>> --- trunk/libavutil/tree.c	Sun Feb 28 20:58:26 2010	(r22118)
>>>>> +++ trunk/libavutil/tree.c	Sun Feb 28 21:48:42 2010	(r22119)
>>>>> @@ -135,6 +135,15 @@ void av_tree_destroy(AVTreeNode *t){
>>>>>      }
>>>>>  }
>>>>>  +void av_tree_destroy_free_elem(AVTreeNode *t){
>>>>> +    if(t){
>>>>> +        av_tree_destroy_free_elem(t->child[0]);
>>>>> +        av_tree_destroy_free_elem(t->child[1]);
>>>>> +        av_free(t->elem);
>>>>> +        av_free(t);
>>>>> +    }
>>>>> +}
>>>>> +
>>>>>  #if 0
>>>>>  void av_tree_enumerate(AVTreeNode *t, void *opaque, int (*cmp)(void 
>>>>> *opaque, void *elem), int (*enu)(void *opaque, void *elem)){
>>>>>      if(t){
>>>>>
>>>>> Modified: trunk/libavutil/tree.h
>>>>> ==============================================================================
>>>>> --- trunk/libavutil/tree.h	Sun Feb 28 20:58:26 2010	(r22118)
>>>>> +++ trunk/libavutil/tree.h	Sun Feb 28 21:48:42 2010	(r22119)
>>>>> @@ -78,5 +78,6 @@ void *av_tree_find(const struct AVTreeNo
>>>>>   */
>>>>>  void *av_tree_insert(struct AVTreeNode **rootp, void *key, int 
>>>>> (*cmp)(void *key, const void *b), struct AVTreeNode **next);
>>>>>  void av_tree_destroy(struct AVTreeNode *t);
>>>>> +void av_tree_destroy_free_elem(struct AVTreeNode *t);
>>>> argh, ive missed this part of your patch
>>>> minor api bump & doxy missing and its the wrong way to do it
>>>> you dont know at all if avfree() is correct and sufficient to free the
>>>> opaque elements.
>>>> av_tree_enumerate() is likely the correct way to free
>>> Ok, but current version of av_tree_enumerate() is commented out and does 
>>> not do what one would expect. What do you think of this patch?
>>>
>>> -Vitor
>>>  libavformat/nut.c    |   11 +++++++++++
>>>  libavformat/nut.h    |    1 +
>>>  libavformat/nutdec.c |    1 +
>>>  libavformat/nutenc.c |    1 +
>>>  libavutil/avutil.h   |    2 +-
>>>  libavutil/tree.c     |   14 ++++++--------
>>>  libavutil/tree.h     |    7 +++++++
>>>  7 files changed, 28 insertions(+), 9 deletions(-)
>>> 073f43adb2344cede149943eef2ce2cae50d7930  nuttree.diff
>>> Index: libavutil/tree.c
>>> ===================================================================
>>> --- libavutil/tree.c	(revision 22135)
>>> +++ libavutil/tree.c	(working copy)
>>> @@ -135,16 +135,14 @@
>>>      }
>>>  }
>>>  -#if 0
>>> -void av_tree_enumerate(AVTreeNode *t, void *opaque, int (*cmp)(void 
>>> *opaque, void *elem), int (*enu)(void *opaque, void *elem)){
>>> -    if(t){
>>> -        int v= cmp ? cmp(opaque, t->elem) : 0;
>>> -        if(v>=0) av_tree_enumerate(t->child[0], opaque, cmp, enu);
>>> -        if(v==0) enu(opaque, t->elem);
>>> -        if(v<=0) av_tree_enumerate(t->child[1], opaque, cmp, enu);
>>> +void av_tree_enumerate(AVTreeNode *t, void *opaque,
>>> +                       void (*enu)(void *opaque, void **elem)){
>>> +    if (t) {
>>> +        av_tree_enumerate(t->child[0], opaque, enu);
>>> +        enu(opaque, &t->elem);
>>> +        av_tree_enumerate(t->child[1], opaque, enu);
>>>      }
>> no, this is broken
>> the only question on the API i had IIRC was if we should allow elements
>> to be removed based on the return code of enu()
>> ruining the API by droping 2/3 of the features is not an option
>
> Ok, so since there is no documentation for it, can you please explain me 
> what av_tree_enumerate() is supposed to do and in what way it is different 
> from
>
> {
>     void *elem  = av_tree_find(t, opaque, cmp, NULL);
>     if (elem)
>         enu(opaque, elem);
> }

av_tree_enumerate() is able to enumerate() ranges not just individual
elements
cmp returns <0 or >0 -> tested element is outside the range to the specified
side, otherwise cmp=0 element is inside the range

suggestions for improvments are welcome
[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20100302/42b11946/attachment.pgp>