[vlc-devel] [PATCH] video_output: simplify setting a new deinterlace mode

[Thomas Guillem]

On Mon, Sep 2, 2019, at 11:16, Steve Lhomme wrote:
> On 2019-09-02 9:57, Thomas Guillem wrote:
> > I don't really see the improvement here. I usually prefer to pass a int used as ternary for that usecase.
> 
> IMO the -1 we currently have doesn't speak for itself. You don't know 
> how the value is going to be used until you read the code. A pointer is 
> clearer IMO for an optional value change. You only set the values you 
> want to change.

Then, use a const bool * please. Otherwise we can think that this function is modifying the bool arg.

> 
> We could also pass vout->p->filter.has_deint, since we already pass 
> vout->p->filter.configuration when the configuration string doesn't change.
> 
> > 
> > On Fri, Aug 30, 2019, at 07:40, Steve Lhomme wrote:
> >> Don't pass a value if we don't want to change it.
> >> ---
> >>   src/video_output/video_output.c | 12 ++++++------
> >>   1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
> >> index 7968a65f88..c77b64cfe7 100644
> >> --- a/src/video_output/video_output.c
> >> +++ b/src/video_output/video_output.c
> >> @@ -744,7 +744,7 @@ typedef struct {
> >>   static void ThreadChangeFilters(vout_thread_t *vout,
> >>                                   const video_format_t *source,
> >>                                   const char *filters,
> >> -                                int deinterlace,
> >> +                                bool *new_deinterlace,
> >>                                   bool is_locked)
> >>   {
> >>       ThreadFilterFlush(vout, is_locked);
> >> @@ -756,8 +756,8 @@ static void ThreadChangeFilters(vout_thread_t *vout,
> >>       vlc_array_init(&array_static);
> >>       vlc_array_init(&array_interactive);
> >>   
> >> -    vout->p->filter.has_deint =
> >> -         deinterlace == 1 || (deinterlace == -1 && vout->p->filter.has_deint);
> >> +    if (new_deinterlace != NULL)
> >> +        vout->p->filter.has_deint = *new_deinterlace;
> > vout->p->filter.has_deint = *
> > 
> > 
> > What about:
> > 
> > if (deinterlace != -1)
> >    vout->p->filter.has_deint = deinterlace == 1;
> > 
> > no ?
> > 
> >>   
> >>       if (vout->p->filter.has_deint)
> >>       {
> >> @@ -903,7 +903,7 @@ static int
> >> ThreadDisplayPreparePicture(vout_thread_t *vout, bool reuse, bool fra
> >>                       }
> >>                   }
> >>                   if (!VideoFormatIsCropArEqual(&decoded->format,
> >> &vout->p->filter.format))
> >> -                    ThreadChangeFilters(vout, &decoded->format,
> >> vout->p->filter.configuration, -1, true);
> >> +                    ThreadChangeFilters(vout, &decoded->format,
> >> vout->p->filter.configuration, NULL, true);
> >>               }
> >>           }
> >>   
> >> @@ -1573,11 +1573,11 @@ static void ThreadControl(vout_thread_t *vout,
> >> vout_control_cmd_t cmd)
> >>           ThreadChangeFilters(vout, NULL,
> >>                               cmd.string != NULL ?
> >>                               cmd.string : vout->p->filter.configuration,
> >> -                            -1, false);
> >> +                            NULL, false);
> >>           break;
> >>       case VOUT_CONTROL_CHANGE_INTERLACE:
> >>           ThreadChangeFilters(vout, NULL, vout->p->filter.configuration,
> >> -                            cmd.boolean ? 1 : 0, false);
> >> +                            &cmd.boolean, false);
> >>           break;
> >>       case VOUT_CONTROL_MOUSE_STATE:
> >>           ThreadProcessMouseState(vout, &cmd.mouse);
> >> -- 
> >> 2.17.1
> >>
> >> _______________________________________________
> >> vlc-devel mailing list
> >> To unsubscribe or modify your subscription options:
> >> https://mailman.videolan.org/listinfo/vlc-devel
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> > 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel