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

Thomas Guillem thomas at gllm.fr
Mon Jun 3 15:42:00 CEST 2019



On Mon, Jun 3, 2019, at 15:04, Steve Lhomme wrote:
> On 2019-06-03 14:55, Thomas Guillem wrote:
> > 
> > On Mon, Jun 3, 2019, at 14:45, Steve Lhomme wrote:
> >> On 2019-06-03 14:39, Thomas Guillem wrote:
> >>> Since spu_Create() is very unlikely to fail (only ENOMEM)
> >>
> >> Seems OK. Although it doesn't check some errors like if no converter is
> >> loaded. Not sure what happens in that case.
> > 
> > Text renderer, converter and scaler are modules that can fail. But each of these spu_t components are checked before being used.
> 
> So you end using a picture that's half converted or half scaled or not 
> using it at all. That seems odd that it's successfully created when it's 
> not functional.

OK, I'll send a new patch set then.

> 
> >>
> >>> ---
> >>>    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;
> >>>        }
> >>> -- 
> >>> 2.20.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
> > 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list