[FFmpeg-soc] libavfilter: port of Imlib2 vhook

[Víctor Paesa]

Hi,

On Dec 21, 2007 10:37 PM, Víctor Paesa wrote:
> Hi,
>
> On Dec 19, 2007 6:50 PM, Vitor <vitor1001 at gmail.com> wrote:
> > Hi
> >
> > Víctor Paesa wrote:
> > > Hi,
> > >
> > > Here I attach a port of the Imlib2 vhook to the libavfilter API for
> > > your kind review/testing.
>
> > > The source contains a few usage examples, usable too as test scenarios.
> >
> > Maybe add a TODO to say that it should one day be in FFmpeg doc...
>
> Done.
>
> > > +const char *const_names[]={
> > > +    "PI",
> > > +    "E",
> > > +    "N",  // frame number (starting at zero)
> > > +    "H",  // frame height
> > > +    "W",  // frame width
> > > +    "h",  // image height
> > > +    "w",  // image width
> > > +    "X",  // previous x
> > > +    "Y",  // previous y
> > > +    NULL
> > > +};
> >
> > This can be a Doxygen comment...
>
> Done.
>
> > > +    imlib2->r = imlib2->g = imlib2->b = 0;
> > > +    imlib2->a = 255;
> > > +    imlib2->x = imlib2->y = imlib2->w = imlib2->h = 0;
> > > +    imlib2->h_a = imlib2->v_a = 0;
> > > +    imlib2->frame_number = 0;
> > > +    imlib2->num_cmd = 0;
> >
> > I think that opaque is alloc'ed with mallocz, so no need to zero it...
>
> Right, fixed.
>
> > > +                    av_log(NULL, AV_LOG_ERROR,
> > > +                           "imlib2 init() cannot load font '%s'\n", str);
> > > +                    return -1;
> >
> > I think you can use av_log(ctx, ...) to give a better error message.
>
> Done.
>
> > > +static int *query_formats(AVFilterLink *link)
> > > +{
> > > +    return avfilter_make_format_list(1, PIX_FMT_RGB32);
> > > +}
> >
> > That's the sad part of this filter, the likely YUV->RGB->YUV
> > conversion... Maybe a libavyuvimlib as a SoC 2010 project? :-)
>
> Hmm, I added it as a wish to the roundup tracker ;-)
>
> > Also, if you don't get any other feedback in two/three days, just commit
> > it. No policy has been decided for when to commit to SoC trees...
>
> OK.
>
>
> On Dec 20, 2007 9:14 PM, Bobby Bingham <uhmmmm at gmail.com> wrote:
> >
> > > Index: allfilters.h
> > > ===================================================================
> > > --- allfilters.h      (revision 1447)
> > > +++ allfilters.h      (working copy)
> > > @@ -27,6 +27,7 @@
> > >  extern AVFilter avfilter_vf_graph;
> > >  extern AVFilter avfilter_vf_graphdesc;
> > >  extern AVFilter avfilter_vf_graphfile;
> > > +extern AVFilter avfilter_vf_imlib2;
> > >  extern AVFilter avfilter_vf_negate;
> > >  extern AVFilter avfilter_vf_overlay;
> > >  extern AVFilter avfilter_vf_passthrough;
> > > Index: avfilter.c
> > > ===================================================================
> > > --- avfilter.c        (revision 1447)
> > > +++ avfilter.c        (working copy)
> > > @@ -340,6 +340,7 @@
> > >      avfilter_register(&avfilter_vf_graph);
> > >      avfilter_register(&avfilter_vf_graphdesc);
> > >      avfilter_register(&avfilter_vf_graphfile);
> > > +    avfilter_register(&avfilter_vf_imlib2);
> > >      avfilter_register(&avfilter_vf_negate);
> > >      avfilter_register(&avfilter_vf_overlay);
> > >      avfilter_register(&avfilter_vf_passthrough);
> >
> > Since the filter is compiled conditionally, shouldn't this also be
> > conditional?
>
> Certainly, fixed.
>
> > Also, the rewritten colorspace negotiation code I just commit requires
> > a minor tweak to the filter.  Attached patch should fix that for
> > vf_imlib2.
> >
> > > Index: Makefile
> > > ===================================================================
> > > --- Makefile  (revision 1447)
> > > +++ Makefile  (working copy)
> > > @@ -23,6 +23,12 @@
> > >
> > >  EXTRALIBS := -L$(BUILD_ROOT)/libavutil -lavutil$(BUILDSUF)
> > > -L$(BUILD_ROOT)/libswscale -lswscale$(BUILDSUF)
> > > -L$(BUILD_ROOT)/libavcodec -lavcodec$(BUILDSUF) $(EXTRALIBS) +ifeq
> > > ($(HAVE_IMLIB2),yes)
> > > +    OBJS-yes += vf_imlib2.o
> > > +    CFLAGS += `imlib2-config --cflags`
> > > +    EXTRALIBS += `imlib2-config --libs`
> > > +endif
> > > +
> >
> > This seems to only work correctly with --enable-shared.  I don't know
> > the build system well enough to suggest the right solution though.
>
> I tried one way.
>
> Thank you for your reviews, attached is a second version, hopefully
> addressing all issues found.

As per discussion in issue 302 in bug tracker, this patch won't be accepted.

I have documented libavfilter's patch acception rules into its README, and
I'll add vf_imlib2 to
http://wiki.multimedia.cx/index.php?title=Interesting_Patches

Regards,
Víctor