[FFmpeg-devel] [PATCH] libavfilter-soc: implement pad filter

[Michael Niedermayer]

On Sat, Oct 03, 2009 at 11:13:20AM +0200, Stefano Sabatini wrote:
> On date Saturday 2009-10-03 03:58:05 +0200, Michael Niedermayer encoded:
> > On Sat, Oct 03, 2009 at 01:38:59AM +0200, Stefano Sabatini wrote:
> > > On date Thursday 2009-10-01 02:28:37 +0200, Michael Niedermayer encoded:
> > > > On Thu, Oct 01, 2009 at 01:00:16AM +0200, Stefano Sabatini wrote:
[...]
> > > 
> > > * to have a cropdetect filter which writes to a log file the detected
> > >   crop area for each frame
> > > 
> > > * to run a new filterchain, applying for each frame the crop values in
> > >   the log file: the crop size will affect the size of each allocated
> > >   frame.
> > > 
> > > I want to clarify this point, as this is actually one of the tricky
> > > points of the pad filter.
> > > 
> > > The geometry of the area of buffer used by a filter needs to be
> > > conveied to the pad filter, because on that depends how the pad filter
> > > will request a new area.
> > 
> > output_width= input_width + h_pad 
> 
> You may have an arbitrarily complicated chain of crop and pad filters,

yes


> each one shrink and expand the used area,

this statement is ambigous, what area? there are several, the allocation
side and the drawing side ...


> but in order to avoid
> crashes

sorry but this is not a basis for discussion, if you cannot explain
the problem i cant do more than reject your patches.
Also no matter how little information is passed through the API,
a crash is at that level always a implementation bug. The filter could
just use a memcpy if it cannot gurantee safe direct rendering.


> you need to keep trace of the position used by the previous
> filter area in relation to the allocated buffer.

thats your claim and i am asking for a single example where this
is needed, a FULL and unamigous example that is free of undefined terms



> 
> > same for height
> > 
> > 
> > > 
> > > Consider this:
> > > 
> > >                     x1        x1+w      exp_w      x1+w+padright
> > > O--->---------------.-----------.---------+ - - - - - - . - - - -
> > > |                   .           .         |             .
> > > |                   .           .         |             .
> > > |                   .           .         |             .
> > > v            +------.-----------.---------+-------------+
> > > |            |      x,y         .         |             |
> > > |            |      +-----------+         |             |
> > > |            |      | previous  |         |             |
> > > |            |      |  filter   |         |             |
> > > |            |      |   area    |         |             |
> > > |            |      |           |         |             |
> > > |            |      +-----------+         |             |
> > > |            |                            |             |
> > > |            +----------------------------+-------------+
> > > |                                         |
> > > +-----------------------------------------+
> > > | exp_h
> > 
> > i do not know what all these variables are, sorry
> > 
> > 
> > > 
> > > |
> > > 
> > > |
> > > 
> > > The x, y, h, w informations are used to detect the dimension of the
> > > buffer to request to the next filter.
> > > 
> > > Check vf_pad.c:get_video_buffer():
> > > 
> > 
> > > static AVFilterPicRef *get_video_buffer(AVFilterLink *link, int perms,
> > >                                         int x, int y, int w, int h, int exp_w, int exp_h)
> > 
> > thats not what we have in svn, not ffmpeg nor soc, so iam sorry but i
> > cant help, i do not know what these parameters are or why they are there.
> 
> I'm attaching again the patches, which correspond to the changes I
> proposed. Anyway as long as we're discussing the design, I believe it
> is more important to focus on the design rather than on the code.
> 

> Also, the get_video_buffer() API changes are related to the pad filter
> implementation, so I considered natural to post them togheter.

The get_video_buffer() changes must be split out and explained why they
are needed. Iam not convinced that they are needed

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- 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-devel/attachments/20091003/716c6a52/attachment.pgp>