[FFmpeg-devel] [PATCH] Add opaque to avfilter_graph_parse()

[Vitor Sessak]

Michael Niedermayer wrote:
> On Fri, May 08, 2009 at 12:52:08PM +0200, Vitor Sessak wrote:
>> V??ctor Paesa wrote:
>>> Hi,
>>> On Sun, April 5, 2009 21:45, Vitor Sessak wrote:
>>>> V??ctor Paesa wrote:
>>>>> Hi,
>>>>>
>>>>> On Sat, March 28, 2009 19:46, V??ctor Paesa wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Sat, March 28, 2009 17:01, Michael Niedermayer wrote:
>>>>>>> On Fri, Mar 27, 2009 at 04:50:43PM +0100, V??ctor Paesa wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I'd like to add a void *opaque to avfilter_graph_parse(), so that
>>>>>>>> it passes it on to avfilter_init_filter(), instead of passing NULL.
>>>>>>>>
>>>>>>>> This extension to the API is not used by ffmpeg.c, but I imagine
>>>>>>>> other applications may want to use the graph parsing facilities,
>>>>>>>> and be happier if it is possible to give some value to opaque
>>>>>>>> (even if the same for every filter in graph).
>>>>>>> humans write plain/text, computers like text/plain
>>>>>>> my mutt is pretty robust in displaying patches with wrong mime types
>>>>>>> inline and colored but this one was too much.
>>>>>> Sorry, I'm afraid the computer I used to send my patches from didn't
>>>>>> had applied this .reg to set the mime types:
>>>>>>
>>>>>> -------------------------------
>>>>>> Windows Registry Editor Version 5.00
>>>>>>
>>>>>> [HKEY_CLASSES_ROOT\.diff]
>>>>>> @="txtfile"
>>>>>> "PerceivedType"="diff"
>>>>>> "Content Type"="text/x-patch"
>>>>>> -------------------------------
>>>>>>
>>>>>> Here are attached again, as text/x-patch
>>>> I'm not against the patch per see, but I'd like someone who followed
>>>> lavfi development from the start to explain why an opaque arg is needed.
>>>>  From memory, I don't remember anything thats uses it and it begs for
>>>> been misused...
>>> Lacking a better explanation from the author, I have the feeling that
>>> the avfilter pipe is effectively isolated: video in, video out, but no way
>>> to report back to the application that there is something interesting in 
>>> one
>>> particular frame.
>>> Imagine a video surveillance application, that detects strangers entering
>>> the door. The opaque pointer would contain a callback function,
>>> for example.
>> Patch ok them, even if I'm not completely happy with the "opaque" concept. 
>> Sorry for the delay...
> 
> id like to make it clear that this patch is not ok for ffmpeg svn
> also id like to repeat that work should be concentrated on moving
> libavfilter code from soc svn to ffmpeg svn.
> Each addition of code to libavfilter in soc will likely make the
> eventual merge MUCH more painfull.
> Its all these little changes that get commited with no thought and no
> justification, all of that will have to be reviewed and almost all
> will have to be reverted before the code could be put in ffmpeg svn.

IMHO we should either:

1- Remove the whole "opaque" stuff
2- Make it usable

This patch is obviously correct assuming we want (2). Do you prefer (1)?

-Vitor