[FFmpeg-devel] Fw: [PATCH] Refactor two near-identical clauses.

[Shlomi Fish]

Hi Michael!

Thanks for your review.

On Thu, 28 Jun 2018 02:38:51 +0200
Michael Niedermayer <michael at niedermayer.cc> wrote:

> On Sun, Jun 17, 2018 at 03:40:19PM +0300, Shlomi Fish wrote:
> > On Sun, 17 Jun 2018 03:05:27 +0200
> > Michael Niedermayer <michael at niedermayer.cc> wrote:
> >   
> > > On Tue, Jun 12, 2018 at 12:53:20PM +0300, Shlomi Fish wrote:  
> > > > This message did not arrive to the list after three submissions.
> > > > 
> > > > Begin forwarded message:
> > > > 
> > > > Date: Tue, 12 Jun 2018 12:42:52 +0300
> > > > From: Shlomi Fish <shlomif at shlomifish.org>
> > > > To: ffmpeg-devel at ffmpeg.org
> > > > Cc: Shlomi Fish <shlomif at shlomifish.org>
> > > > Subject: [PATCH] Refactor two near-identical clauses.
> > > > 
> > > > 
> > > > Placed under the Expat licence . All tests pass.
> > > > ---
> > > >  libavfilter/vf_weave.c | 33 ++++++++++++++-------------------
> > > >  1 file changed, 14 insertions(+), 19 deletions(-)
> > > > 
> > > > diff --git a/libavfilter/vf_weave.c b/libavfilter/vf_weave.c
> > > > index 037f5d1cf2..be371201e1 100644
> > > > --- a/libavfilter/vf_weave.c
> > > > +++ b/libavfilter/vf_weave.c
> > > > @@ -23,6 +23,7 @@
> > > >  #include "libavutil/pixdesc.h"
> > > >  #include "avfilter.h"
> > > >  #include "internal.h"
> > > > +#include <stdbool.h>
> > > >  
> > > >  typedef struct WeaveContext {
> > > >      const AVClass *class;
> > > > @@ -84,6 +85,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> > > > *in) AVFilterLink *outlink = ctx->outputs[0];
> > > >      AVFrame *out;
> > > >      int i;
> > > > +    bool weave;
> > > > +    int field1, field2;
> > > >  
> > > >      if (!s->prev) {
> > > >          s->prev = in;
> > > > @@ -98,26 +101,18 @@ static int filter_frame(AVFilterLink *inlink,
> > > > AVFrame *in) }
> > > >      av_frame_copy_props(out, in);
> > > >  
> > > > +    weave = (s->double_weave && !(inlink->frame_count_out & 1));
> > > > +    field1 = s->first_field * weave;
> > > > +    field2 = s->first_field * !weave;
> > > >      for (i = 0; i < s->nb_planes; i++) {
> > > > -        if (s->double_weave && !(inlink->frame_count_out & 1)) {
> > > > -            av_image_copy_plane(out->data[i] + out->linesize[i] *
> > > > s->first_field,    
> > > 
> > > this seems to be corrupted by line breaks
> > >   
> > 
> > Well, the git send-email email was silently dropped three times... See:
> > 
> > http://www.shlomifish.org/Files/files/code/0001-Refactor-two-near-identical-clauses.patch
> > 
> > also attached here. Email has sadly become unreliable.
> >   
> > > [...]
> > >   
> > 
> >   
> 
> >  vf_weave.c |   33 ++++++++++++++-------------------
> >  1 file changed, 14 insertions(+), 19 deletions(-)
> > f5a0afe735e322c2539a11dd7d8b28534d96c99c
> > 0001-Refactor-two-near-identical-clauses.patch From
> > b6678799848297cd7079085035259baf6d8c54f0 Mon Sep 17 00:00:00 2001 From:
> > Shlomi Fish <shlomif at shlomifish.org> Date: Fri, 25 May 2018 23:44:54 +0300
> > Subject: [PATCH] Refactor two near-identical clauses.
> > 
> > Placed under the Expat licence . All tests pass.
> > ---
> >  libavfilter/vf_weave.c | 33 ++++++++++++++-------------------
> >  1 file changed, 14 insertions(+), 19 deletions(-)
> > 
> > diff --git a/libavfilter/vf_weave.c b/libavfilter/vf_weave.c
> > index 037f5d1cf2..be371201e1 100644
> > --- a/libavfilter/vf_weave.c
> > +++ b/libavfilter/vf_weave.c
> > @@ -23,6 +23,7 @@
> >  #include "libavutil/pixdesc.h"
> >  #include "avfilter.h"
> >  #include "internal.h"
> > +#include <stdbool.h>
> >  
> >  typedef struct WeaveContext {
> >      const AVClass *class;
> > @@ -84,6 +85,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
> >      AVFilterLink *outlink = ctx->outputs[0];
> >      AVFrame *out;
> >      int i;
> > +    bool weave;
> > +    int field1, field2;
> >  
> >      if (!s->prev) {
> >          s->prev = in;
> > @@ -98,26 +101,18 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> > *in) }
> >      av_frame_copy_props(out, in);
> >  
> > +    weave = (s->double_weave && !(inlink->frame_count_out & 1));
> > +    field1 = s->first_field * weave;
> > +    field2 = s->first_field * !weave;  
> 
> if first_field is 0 both field1 and 2 are 0 while !s->first_field was not 0
> but gets replaced by it
> am i missing something ?
> 

so first_field is a boolean? I guess I can amend the code to the effect.

> stdbool seems unneeded
> 

I prefer to use it, but I guess I can avoid it.

> and what is "Placed under the Expat licence" supposed to mean ?
> the file is under LGPL
> 

It means I licence my changes under Expat, on top of the original terms of the
original code.

Regards,

	Shlomi

> 
> [...]
> 
> 



-- 
-----------------------------------------------------------------
Shlomi Fish       http://www.shlomifish.org/
https://youtu.be/n6KAGqjdmsk - “Hurt Me Tomorrow”

Tomorrow never dies, unless Chuck Norris volunteers to take it out of its
misery.
    — http://www.shlomifish.org/humour/bits/facts/Chuck-Norris/

Please reply to list if it's a mailing list post - http://shlom.in/reply .