Re: [FFmpeg-devel] [PATCH] lavfi: get rid of FF_INTERNAL_FIELDS

From: Anton Khirnov <anton@khirnov.net>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] lavfi: get rid of FF_INTERNAL_FIELDS
Date: Tue, 31 Jan 2023 14:50:28 +0100
Message-ID: <167517302897.4503.15130184316413800795@lain.khirnov.net> (raw)
In-Reply-To: <Y9e6poV1qZ+t2TSs@phare.normalesup.org>

Quoting Nicolas George (2023-01-30 13:40:06)
> Anton Khirnov (12023-01-30):
> > This hack is used to limit the visibility of some AVFilterLink fields to
> > only certain files. Replace it with the same pattern that is used e.g.
> > in lavf AVStream/FFstream and avoid exposing these internal fields in a
> > public header completely.
> > ---
> >  libavfilter/avfilter.c       | 191 +++++++++++++++++++++--------------
> >  libavfilter/avfilter.h       |  45 ---------
> >  libavfilter/avfiltergraph.c  |   9 +-
> >  libavfilter/buffersink.c     |   8 +-
> >  libavfilter/link_internal.h  |  69 +++++++++++++
> >  libavfilter/tests/filtfmts.c |   9 +-
> >  6 files changed, 196 insertions(+), 135 deletions(-)
> >  create mode 100644 libavfilter/link_internal.h
> 
> This makes the code more verbose, less readable and harder to maintain,

I find this a significant improvement in code quality, making it easier
to maintain.

> so no thanks.
> 
> If you find a solution that does not require us to remember which field
> is private and with field is public to prefix them with link-> or li->,
> it would not have this issue; avoiding this requirement was a prime goal
> of the current implementation. At least you did not add an indirection
> like on some other places.

Making it obvious which field is private and which is public is a
feature, not a bug.

I'll also note that
- we've been switching to this precise pattern everywhere else in the
  project
- the most prolific lavfi contributor recently (Paul) has no objections
  to this patch
- the second most prolific lavfi contributor recently (Andreas) told me
  on IRC he planned to write this same patch himself

So if you insist on objecting to this patch, it seems to me that a vote
would be in order.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".