[FFmpeg-devel] [PATCH 1/2] avcodec: Add interface to motion estimation

Ronald S. Bultje rsbultje at gmail.com
Mon Aug 31 13:11:38 CEST 2015


Hi Michael,

On Sun, Aug 30, 2015 at 10:02 PM, Michael Niedermayer <
michael at niedermayer.cc> wrote:

> On Sun, Aug 30, 2015 at 08:53:37PM -0400, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Sat, Aug 29, 2015 at 10:23 PM, Michael Niedermayer <michaelni at gmx.at>
> > wrote:
> >
> > > From: Michael Niedermayer <michael at niedermayer.cc>
> > >
> > > This is needed for vf_mcfps, no codec related structs are part of the
> > > public interface
> > >
> > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > ---
> > >  libavcodec/Makefile   |    2 +-
> > >  libavcodec/avme.c     |  138
> > > +++++++++++++++++++++++++++++++++++++++++++++++++
> > >  libavcodec/avme.h     |   30 +++++++++++
> > >  libavcodec/internal.h |    2 +
> > >  libavcodec/snow.c     |   30 +++++++++++
> > >  libavcodec/version.h  |    2 +-
> > >  6 files changed, 202 insertions(+), 2 deletions(-)
> > >  create mode 100644 libavcodec/avme.c
> > >  create mode 100644 libavcodec/avme.h
> >
> >
> > This is just wrapper code. It may fix an interface issue, but not the
> > implementation issue. Can you make ME available without depending on
> snow?
>
> the currently used ME code is part of snow, its a integral part of snow


So extract it.

> I can't imagine this is hard, don't we have an mpeg encoder, doesn't that
> > use ME also?
>
> snow has iterative OBMC based motion estimation, mpeg does neither
> have iterative nor OBMC based ME
>
> If someone extracts that out of snow then by the time mcfps maybe
> will use its own motion estimation independant of libavcodec. Or
> maybe it wont i dont know. but if anything then the ME code
> used is not good enough, and the mpeg code is worse. Also the code
> has different goals, libavcodec ME code tries to find good matches
> for compression, it doesnt really truly matter if that matches the
> actual motion but for frame interpolation a single wrong motion
> vector can cause severe artifacts even if the block from it matches
> pixel wise very well
> so iam a bit reluctant to do API and implementation work around
> the ME interface ATM, my time would be better spend in improving and
> testing mcfps IMO
>

My time would be better spend than cleaning up after you. Who will clean
this up? Nobody?

what seems the most pressing need to me is to make it easier for other
> developers to test and work on the code. Putting it in the main
> development branch aka git master would achive that. Which is why
> iam suggesting to do that.
> Do you agree ?


No. You have this tendency to add significant hacks to the codebase, that
take a team of experienced developers years to clean up. For some reason,
we give you special treatment to add hacks all over the place, just so you
can get your work done.

We're about to add FF_QSCALE_TYPE_MPEG1 to libavutil, and it's part of our
intra-library ABI so we can't change it unless we bump majors, even though
it's in internal.h. This is all leftover crap from years ago that will
never truly be cleaned up. We're stuck with it, likely forever. This is
exactly the same. Let's not do this again and feel incredibly sorry about
it years from now.

How about we teach you design. Please extract OBMC properly from snow, make
it a proper interface whose implementation does not depend on source files
called snow*, can be compiled as an independent feature ("obmc") on which
snow encoding depends, and whose implementation does not depend on symbols
only available under HAVE_SNOW*, etc. Please try to learn to do these kind
of things cleanly. Once you've learned how to do it, you get much better at
it and eventually it'll come naturally with no effort. You'll be a better
engineer.

Ronald


More information about the ffmpeg-devel mailing list