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

[wm4]

On Mon, 31 Aug 2015 07:11:38 -0400
"Ronald S. Bultje" <rsbultje at gmail.com> wrote:

> 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.

+1

I'm also worried about the Snow codec tentacling around libavutil. On
the other hand, a good self-contained motion estimation
API/implementation sounds like a worthy goal to put effort in.