[FFmpeg-devel] [PATCH] avfilter: initial macroblock types export and visualization

[Ronald S. Bultje]

Hi,

On Thu, Nov 2, 2017 at 7:52 AM, Paul B Mahol <onemda at gmail.com> wrote:

> On 11/2/17, Michael Niedermayer <michael at niedermayer.cc> wrote:
> > Hi
> >
> > On Sat, Oct 28, 2017 at 07:43:05AM -0400, Ronald S. Bultje wrote:
> >> Hi,
> >>
> >> On Fri, Oct 27, 2017 at 10:14 PM, Michael Niedermayer <
> >> michael at niedermayer.cc> wrote:
> >>
> >> > On Fri, Oct 27, 2017 at 10:03:54PM +0200, Paul B Mahol wrote:
> >> > > Signed-off-by: Paul B Mahol <onemda at gmail.com>
> >> > > ---
> >> > >  libavcodec/mpegvideo.c     |  10 +++++
> >> > >  libavfilter/vf_codecview.c | 105 ++++++++++++++++++++++++++++++
> >> > +++++++++++++++
> >> > >  libavutil/frame.h          |   4 ++
> >> > >  3 files changed, 119 insertions(+)
> >> >
> >> > First, thanks for working on this.
> >> >
> >> >
> >> > [...]
> >> >
> >> > > diff --git a/libavutil/frame.h b/libavutil/frame.h
> >> > > index fef558ea2f..8481dc080b 100644
> >> > > --- a/libavutil/frame.h
> >> > > +++ b/libavutil/frame.h
> >> > > @@ -141,6 +141,10 @@ enum AVFrameSideDataType {
> >> > >       * metadata key entry "name".
> >> > >       */
> >> > >      AV_FRAME_DATA_ICC_PROFILE,
> >> > > +    /**
> >> > > +     * Macroblock types exported by some codecs.
> >> > > +     */
> >> > > +    AV_FRAME_DATA_MACROBLOCK_TYPES,
> >> > >  };
> >> > >
> >> >
> >> > This makes the internal data of the decoder part of the ABI and API of
> >> > libavcodec and libavfilter
> >> > and its undocumented
> >> >
> >> > do people prefer to make the internal data part of the ABI, document
> >> > it and ensure it does not change till the next bump
> >> >
> >> [..]
> >>
> >> > or is there some other option iam missing ?
> >> >
> >>
> >> I noted this on IRC also. A third option is to not document it and
> >> consider
> >> it codec-specific.
> >>
> >> (Codec-specific implies "ABI/API unstable" and subject to change -
> "don't
> >> mix versions".)
> >
> > If you say the interface is "ABI/API unstable" then we cannot use it
> > from another lib like libavfilter. So this would not work.
>
> I disagree.


I'd like to speak in solutions, not problems. We can simply prepend a
version number to the flags and ensure versions match. That makes it easy
to break "ABI" (and if versions don't match, simply display an error
message and tell the user to not mix versions since this is considered an
unstable feature).

By no means can we set the current ABI of this feature in stone, we're
trying to fix it, not keep it. I appreciate Paul's effort to moving this
forward and think his work pushes it in the right direction.

Ronald