[FFmpeg-devel] [PATCH] libavfilter-soc: implement pad filter
Stefano Sabatini
stefano.sabatini-lala
Sat Oct 3 11:13:20 CEST 2009
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:
> > > > Sorry for the slow reply.
> > > [...]
> > > > 2) the buffer obtained by the pad filter with get_video_buffer() is
> > > > inverted and passed to the previous filter, as Vitor and IIUC Michael
> > > > proposed (check the patch: fix-vflip-logic+vitor.patch).
> > > >
> > > > Consider the filterchain: "crop=0:0:WIDTH:100, vflip"
> > > >
> > > > In this scenario the crop filter will take the top 100 lines of the
> > > > image in input.
> > > >
> > > >
> > > > This is the picref received in input by vflip.c:get_video_buffer():
> > > >
> > > > [fig. 4]
> > > >
> > > > O----------+
> > > > | crop |
> > > > | area |
> > > > | |
> > > > v |
> > > > +----------+
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > +----------+
> > > >
> > > > The vflip filter moves the origin to the bottom left corner of the
> > > > input area, and inverts the linesizes, so this is what
> > > > vf_crop.c:get_video_buffer() obtains:
> > > >
> > > > [fig. 5]
> > > >
> > > > .
> > > > .
> > > > .
> > > > +----------+
> > > > ^ crop |
> > > > | area |
> > > > | |
> > > > | |
> > > > O----------+
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > +----------+
> > >
> > >
> > > i would have expected:
> > >
> > > > +----------+
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > | |
> > > > O----------+
> >
> > So the correct solution would be to invert the reference of the
> > *whole* allocated buffer.
> >
> > The problem is that the vflip filter, as well as all the other ones,
> > has no idea of the bottom left corner of the allocated buffer. Maybe
> > if the position of the returned buffer in the allocated buffer would
> > be known (for example it could be stored in the picref itself) than it
> > would be possible to perform the correct inversion.
> >
> > I'll try to give a try at this approach tomorrow.
> >
> > > about the crop area shown in your figs, i dont understand how the
> > > get_video_buffer() code could even know about croping that is done
> > > at later stages. As an example assume a cropdetect filter that analzes
> > > the picture content and then crops the black borders, it could in no way
> > > know about where and what to crop when allocating the buffers
> >
> > Michael, with the actual architecture that wouldn't be even possible.
> >
> > The whole recursive get_video_buffer()+pad filter thing may work only
> > if a filter can specify to the next filter *the exact area of the
> > buffer it will use*, which is specified by the x, y, w, and h
> > parameters which are passed to the get_video_buffer() function.
>
> in svn:
> AVFilterPicRef *(*get_video_buffer)(AVFilterLink *link, int perms);
> more precissely no x,y,w,h
> width & height clearly would have to be added, for others i dont know
> you arent explaining things much and what you do try to explain i cant
> make too much sense of ...
>
> thus to summarize, (please correct me if iam wrong)
> * there is NO problem in the current system and vflip
> * you added parameters to your private get_video_buffer() that break vflip
> * i am not aware of why these parameters would be needed or usefull
> * mplayer can do direct rendering, copyless vflip, pad and crop.
>
>
> >
> > Indeed when the filter writes to the buffer, it has been already
> > allocated, the best you can do in your scenario would be a 2-pass
> > processing e.g.:
>
> if its not possible to do this in 1pass, not even with extra memcopies
> then i think thats a quite nasty limitation and id like to know where
> this limitation comes from.
>
> also i like to repeat what i said many times, libavfilter MUST be moved
> into main svn as soon as possible
>
>
> >
> > * 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,
each one shrink and expand the used area, but in order to avoid
crashes you need to keep trace of the position used by the previous
filter area in relation to the allocated buffer.
> 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.
Regards.
--
FFmpeg = Frightening and Fundamental MultiPurpose Easy Guru
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lavfi-recursive-get-video-buffer.patch
Type: text/x-diff
Size: 16400 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091003/4a655cde/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pad-implement.patch
Type: text/x-diff
Size: 16764 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091003/4a655cde/attachment-0001.patch>
More information about the ffmpeg-devel
mailing list