[FFmpeg-soc] libavfilter: port of Imlib2 vhook

[Víctor Paesa]

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.

Regards,
Víctor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg.libavfilter.imlib2.2.diff
Type: text/x-patch
Size: 18267 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20071221/f7c18b87/attachment.bin>