[FFmpeg-devel] [PATCH] Move link_dpad and link_spad macros from avfilter.c to internal.h, so

[Michael Niedermayer]

On Sat, Sep 04, 2010 at 01:43:27PM +0200, Stefano Sabatini wrote:
> On date Saturday 2010-09-04 01:38:38 +0200, Michael Niedermayer encoded:
> > On Fri, Sep 03, 2010 at 04:47:36PM +0200, Stefano Sabatini wrote:
> > > On date Tuesday 2010-08-24 12:26:05 +0200, Stefano Sabatini encoded:
> > > > On date Tuesday 2010-07-20 18:42:44 +0200, Michael Niedermayer encoded:
> > > > > On Mon, Jul 19, 2010 at 06:19:56PM +0200, Stefano Sabatini wrote:
> > > > > > ---
> > > > > >  libavfilter/avfilter.c |    4 ----
> > > > > >  libavfilter/internal.h |    4 ++++
> > > > > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> > > > > > index dc8f90d..e81fa48 100644
> > > > > > --- a/libavfilter/avfilter.c
> > > > > > +++ b/libavfilter/avfilter.c
> > > > > > @@ -41,10 +41,6 @@ const char *avfilter_license(void)
> > > > > >      return LICENSE_PREFIX FFMPEG_LICENSE + sizeof(LICENSE_PREFIX) - 1;
> > > > > >  }
> > > > > >  
> > > > > > -/** helper macros to get the in/out pad on the dst/src filter */
> > > > > > -#define link_dpad(link)     link->dst-> input_pads[link->dstpad]
> > > > > > -#define link_spad(link)     link->src->output_pads[link->srcpad]
> > > > > > -
> > > > > 
> > > > > iam against such obfuscation macros
> > > > 
> > > > So you either remove them or make them available in other files as
> > > > well.
> > > > 
> > > > Maybe the link_dstpad / link_srcpad names would read better?
> > > > 
> > > > As for me I surely find more readable:
> > > > AVFilterPad *pad = link_dstpad(link);
> > > > rather than:
> > > > AVFilterPad *pad = link->dst->input_pads[link->dstpad];
> > > 
> > > Updated with the names: FF_LINK_DSTPAD/SRCPAD.
> > > 
> > > I'll apply in three days if I see no objections.
> > 
> > i already said iam against the macros
> > also iam against such macros in general not just here, this is not clean
> > code.
> > 
> > If accessing a field requires a huge line of code and this is used
> > hundereds of times then there is a problem with the design, and the
> > fix is not to hide this behind a macro.  And besides it should be a
> > static inline function not a macro if the design cant be fixed or
> > the fix has worse side effects than this
> 
> This is not a problem of efficiency, it is a problem of readability.

you say this as if it makes a difference. Iam failing to see a difference
though, if a design requires huge lines the design possibly needs to be
improved. If theres a speed or rather a readability problem with that doesnt
really change it.


> So I suggest to take the static inline path, I don't think the design
> should be changed.

i will consider this once i see arguments in favor of it compared to
replacing the array index int by a pointer

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100904/8bccbe57/attachment.pgp>