[vlc-devel] [PATCH] vout: make spu_t mandatory

[Hugo Beauzée-Luyssen]

On Mon, Jun 3, 2019, at 2:43 PM, Hugo Beauzée-Luyssen wrote:
> On Mon, Jun 3, 2019, at 2:40 PM, Thomas Guillem wrote:
> > Since spu_Create() is very unlikely to fail (only ENOMEM)
> > ---
> >  src/video_output/video_output.c | 39 ++++++++++++++-------------------
> >  1 file changed, 16 insertions(+), 23 deletions(-)
> > 
> > diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
> > index c941a3ded9..2fc98e0662 100644
> > --- a/src/video_output/video_output.c
> > +++ b/src/video_output/video_output.c
> > @@ -273,8 +273,7 @@ int vout_RegisterSubpictureChannel( vout_thread_t *vout )
> >      int channel = VOUT_SPU_CHANNEL_AVAIL_FIRST;
> >  
> >      vlc_mutex_lock(&vout->p->spu_lock);
> > -    if (vout->p->spu)
> > -        channel = spu_RegisterChannel(vout->p->spu);
> > +    channel = spu_RegisterChannel(vout->p->spu);
> >      vlc_mutex_unlock(&vout->p->spu_lock);
> >  
> >      return channel;
> > @@ -284,8 +283,7 @@ void vout_SetSubpictureClock( vout_thread_t *vout, 
> > vlc_clock_t *clock )
> >  {
> >      assert(!vout->p->dummy);
> >      vlc_mutex_lock(&vout->p->spu_lock);
> > -    if (vout->p->spu)
> > -        spu_clock_Set(vout->p->spu, clock);
> > +    spu_clock_Set(vout->p->spu, clock);
> >      vlc_mutex_unlock(&vout->p->spu_lock);
> >  }
> >  
> > @@ -295,8 +293,7 @@ void vout_FlushSubpictureChannel( vout_thread_t 
> > *vout, int channel )
> >      assert(!sys->dummy);
> >  
> >      vlc_mutex_lock(&sys->spu_lock);
> > -    if (sys->spu != NULL)
> > -        spu_ClearChannel(vout->p->spu, channel);
> > +    spu_ClearChannel(vout->p->spu, channel);
> >      vlc_mutex_unlock(&sys->spu_lock);
> >  }
> >  
> > @@ -305,8 +302,7 @@ void vout_SetSpuHighlight( vout_thread_t *vout,
> >  {
> >      assert(!vout->p->dummy);
> >      vlc_mutex_lock(&vout->p->spu_lock);
> > -    if (vout->p->spu)
> > -        spu_SetHighlight(vout->p->spu, spu_hl);
> > +    spu_SetHighlight(vout->p->spu, spu_hl);
> >      vlc_mutex_unlock(&vout->p->spu_lock);
> >  }
> >  
> > @@ -584,8 +580,7 @@ void vout_ControlChangeSubSources(vout_thread_t 
> > *vout, const char *filters)
> >  {
> >      assert(!vout->p->dummy);
> >      vlc_mutex_lock(&vout->p->spu_lock);
> > -    if (likely(vout->p->spu != NULL))
> > -        spu_ChangeSources(vout->p->spu, filters);
> > +    spu_ChangeSources(vout->p->spu, filters);
> >      vlc_mutex_unlock(&vout->p->spu_lock);
> >  }
> >  
> > @@ -593,16 +588,13 @@ void vout_ControlChangeSubFilters(vout_thread_t 
> > *vout, const char *filters)
> >  {
> >      assert(!vout->p->dummy);
> >      vlc_mutex_lock(&vout->p->spu_lock);
> > -    if (likely(vout->p->spu != NULL))
> > -        spu_ChangeFilters(vout->p->spu, filters);
> > +    spu_ChangeFilters(vout->p->spu, filters);
> >      vlc_mutex_unlock(&vout->p->spu_lock);
> >  }
> >  
> >  void vout_ChangeSubMargin(vout_thread_t *vout, int margin)
> >  {
> >      assert(!vout->p->dummy);
> > -    if (unlikely(vout->p->spu == NULL))
> > -        return;
> >  
> >      vlc_mutex_lock(&vout->p->spu_lock);
> >      spu_ChangeMargin(vout->p->spu, margin);
> > @@ -1307,11 +1299,8 @@ static void vout_FlushUnlocked(vout_thread_t 
> > *vout, bool below,
> >      vlc_clock_SetDelay(vout->p->clock, vout->p->delay);
> >  
> >      vlc_mutex_lock(&vout->p->spu_lock);
> > -    if (vout->p->spu)
> > -    {
> > -        spu_clock_Reset(vout->p->spu);
> > -        spu_clock_SetDelay(vout->p->spu, vout->p->spu_delay);
> > -    }
> > +    spu_clock_Reset(vout->p->spu);
> > +    spu_clock_SetDelay(vout->p->spu, vout->p->spu_delay);
> >      vlc_mutex_unlock(&vout->p->spu_lock);
> >  }
> >  
> > @@ -1374,8 +1363,7 @@ void vout_ChangeSpuDelay(vout_thread_t *vout, 
> > vlc_tick_t delay)
> >  {
> >      assert(!vout->p->dummy);
> >      vlc_mutex_lock(&vout->p->spu_lock);
> > -    if (vout->p->spu)
> > -        spu_clock_SetDelay(vout->p->spu, delay);
> > +    spu_clock_SetDelay(vout->p->spu, delay);
> >      vout->p->spu_delay = delay;
> >      vlc_mutex_unlock(&vout->p->spu_lock);
> >  }
> > @@ -1726,7 +1714,6 @@ void vout_Close(vout_thread_t *vout)
> >  
> >      vlc_mutex_lock(&sys->spu_lock);
> >      spu_Destroy(sys->spu);
> > -    sys->spu = NULL;
> >      vlc_mutex_unlock(&sys->spu_lock);
> >  
> >      vout_Release(vout);
> > @@ -1827,8 +1814,13 @@ vout_thread_t *vout_Create(vlc_object_t *object)
> >      vout_statistic_Init(&sys->statistic);
> >  
> >      /* Initialize subpicture unit */
> > -    vlc_mutex_init(&sys->spu_lock);
> >      sys->spu = spu_Create(vout, vout);
> > +    if (!sys->spu)
> > +    {
> > +        vlc_object_delete(vout);
> > +        return NULL;
> > +    }
> > +    vlc_mutex_init(&sys->spu_lock);
> >  
> >      vout_control_Init(&sys->control);
> >  
> > @@ -1846,6 +1838,7 @@ vout_thread_t *vout_Create(vlc_object_t *object)
> >      sys->display_cfg.window = vout_display_window_New(vout);
> >      if (sys->display_cfg.window == NULL) {
> >          spu_Destroy(sys->spu);
> > +        vlc_mutex_destroy(&sys->spu_lock);
> >          vlc_object_delete(vout);
> >          return NULL;
> >      }
> 
> Shouldn't spu allowed to be NULL in case VLC is run with --no-spu?
> 

(Granted, for now --no-spu will only reject the tracks and a module will be loaded regardless, but I'm not sure that behavior is wanted in the long term)

-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr