[vlc-devel] [PATCH 2/5] vout: lock only when needed
Thomas Guillem
thomas at gllm.fr
Mon Jun 3 15:59:57 CEST 2019
On Mon, Jun 3, 2019, at 15:57, Steve Lhomme wrote:
> On 2019-06-03 15:41, Thomas Guillem wrote:
> > ---
> > src/video_output/video_output.c | 72 +++++++++++++++++++++------------
> > 1 file changed, 47 insertions(+), 25 deletions(-)
> >
> > diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
> > index d9f91e9e68..0a89f11651 100644
> > --- a/src/video_output/video_output.c
> > +++ b/src/video_output/video_output.c
> > @@ -259,12 +259,14 @@ void vout_PutSubpicture( vout_thread_t *vout, subpicture_t *subpic )
> > vout_thread_sys_t *sys = vout->p;
> > assert(!sys->dummy);
> >
> > - vlc_mutex_lock(&sys->spu_lock);
> > if (sys->spu != NULL)
> > + {
> > + vlc_mutex_lock(&sys->spu_lock);
> > spu_PutSubpicture(sys->spu, subpic);
> > + vlc_mutex_unlock(&sys->spu_lock);
> > + }
> > else
> > subpicture_Delete(subpic);
> > - vlc_mutex_unlock(&sys->spu_lock);
> > }
> >
> > int vout_RegisterSubpictureChannel( vout_thread_t *vout )
> > @@ -272,10 +274,12 @@ int vout_RegisterSubpictureChannel( vout_thread_t *vout )
> > assert(!vout->p->dummy);
> > int channel = VOUT_SPU_CHANNEL_AVAIL_FIRST;
> >
> > - vlc_mutex_lock(&vout->p->spu_lock);
> > if (vout->p->spu)
> > + {
> > + vlc_mutex_lock(&vout->p->spu_lock);
> > channel = spu_RegisterChannel(vout->p->spu);
> > - vlc_mutex_unlock(&vout->p->spu_lock);
> > + vlc_mutex_unlock(&vout->p->spu_lock);
> > + }
> >
> > return channel;
> > }
> > @@ -283,10 +287,12 @@ int vout_RegisterSubpictureChannel( vout_thread_t *vout )
> > 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)
> > + {
> > + vlc_mutex_lock(&vout->p->spu_lock);
> > spu_clock_Set(vout->p->spu, clock);
> > - vlc_mutex_unlock(&vout->p->spu_lock);
> > + vlc_mutex_unlock(&vout->p->spu_lock);
> > + }
> > }
> >
> > void vout_FlushSubpictureChannel( vout_thread_t *vout, int channel )
> > @@ -294,20 +300,24 @@ void vout_FlushSubpictureChannel( vout_thread_t *vout, int channel )
> > vout_thread_sys_t *sys = vout->p;
> > assert(!sys->dummy);
> >
> > - vlc_mutex_lock(&sys->spu_lock);
> > if (sys->spu != NULL)
> > + {
> > + vlc_mutex_lock(&sys->spu_lock);
> > spu_ClearChannel(vout->p->spu, channel);
> > - vlc_mutex_unlock(&sys->spu_lock);
> > + vlc_mutex_unlock(&sys->spu_lock);
> > + }
> > }
> >
> > void vout_SetSpuHighlight( vout_thread_t *vout,
> > const vlc_spu_highlight_t *spu_hl )
> > {
> > assert(!vout->p->dummy);
> > - vlc_mutex_lock(&vout->p->spu_lock);
> > if (vout->p->spu)
> > + {
> > + vlc_mutex_lock(&vout->p->spu_lock);
> > spu_SetHighlight(vout->p->spu, spu_hl);
> > - vlc_mutex_unlock(&vout->p->spu_lock);
> > + vlc_mutex_unlock(&vout->p->spu_lock);
> > + }
> > }
> >
> > /**
> > @@ -583,19 +593,23 @@ void vout_ControlChangeFilters(vout_thread_t *vout, const char *filters)
> > 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))
> > + {
> > + vlc_mutex_lock(&vout->p->spu_lock);
> > spu_ChangeSources(vout->p->spu, filters);
> > - vlc_mutex_unlock(&vout->p->spu_lock);
> > + vlc_mutex_unlock(&vout->p->spu_lock);
> > + }
> > }
> >
> > 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))
> > + {
> > + vlc_mutex_lock(&vout->p->spu_lock);
> > spu_ChangeFilters(vout->p->spu, filters);
> > - vlc_mutex_unlock(&vout->p->spu_lock);
> > + vlc_mutex_unlock(&vout->p->spu_lock);
> > + }
> > }
> >
> > void vout_ChangeSubMargin(vout_thread_t *vout, int margin)
> > @@ -1070,7 +1084,8 @@ static int ThreadDisplayRenderPicture(vout_thread_t *vout, bool is_forced)
> >
> > video_format_t fmt_spu_rot;
> > video_format_ApplyRotation(&fmt_spu_rot, &fmt_spu);
> > - subpicture_t *subpic = spu_Render(sys->spu,
> > + subpicture_t *subpic = !sys->spu ? NULL
> > + : spu_Render(sys->spu,
> > subpicture_chromas, &fmt_spu_rot,
> > &vd->source, system_now,
> > render_subtitle_date, sys->spu_rate,
> > @@ -1306,13 +1321,13 @@ static void vout_FlushUnlocked(vout_thread_t *vout, bool below,
> > vlc_clock_Reset(vout->p->clock);
> > vlc_clock_SetDelay(vout->p->clock, vout->p->delay);
> >
> > - vlc_mutex_lock(&vout->p->spu_lock);
> > if (vout->p->spu)
> > {
> > + vlc_mutex_lock(&vout->p->spu_lock);
> > spu_clock_Reset(vout->p->spu);
> > spu_clock_SetDelay(vout->p->spu, vout->p->spu_delay);
> > + vlc_mutex_unlock(&vout->p->spu_lock);
>
> Apart from this one that makes 2 calls (non atomic), all the others seem
> to be equivalent to the lock that already exists in the spu. Maybe one
> of them can be removed ?
maybe
>
> > }
> > - vlc_mutex_unlock(&vout->p->spu_lock);
> > }
> >
> > void vout_Flush(vout_thread_t *vout, vlc_tick_t date)
> > @@ -1373,19 +1388,24 @@ void vout_ChangeRate(vout_thread_t *vout, float rate)
> > 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)
> > + {
> > + vlc_mutex_lock(&vout->p->spu_lock);
> > spu_clock_SetDelay(vout->p->spu, delay);
> > - vout->p->spu_delay = delay;
> > - vlc_mutex_unlock(&vout->p->spu_lock);
> > + vout->p->spu_delay = delay;
> > + vlc_mutex_unlock(&vout->p->spu_lock);
> > + }
> > }
> >
> > void vout_ChangeSpuRate(vout_thread_t *vout, float rate)
> > {
> > assert(!vout->p->dummy);
> > - vlc_mutex_lock(&vout->p->spu_lock);
> > - vout->p->spu_rate = rate;
> > - vlc_mutex_unlock(&vout->p->spu_lock);
> > + if (vout->p->spu)
> > + {
> > + vlc_mutex_lock(&vout->p->spu_lock);
> > + vout->p->spu_rate = rate;
> > + vlc_mutex_unlock(&vout->p->spu_lock);
> > + }
>
> This change doesn't seem necessary. If the goal is to protect the
> variable maybe it could be a vlc_atomic_float ?
No, rate and delay should go into the spu_t. But First I need to handle that for each channels. cf. Roland patches about dual subtitles.
>
> > }
> >
> > static void ThreadProcessMouseState(vout_thread_t *vout,
> > @@ -1725,10 +1745,12 @@ void vout_Close(vout_thread_t *vout)
> > vout_control_Dead(&sys->control);
> > vout_chrono_Clean(&sys->render);
> >
> > - vlc_mutex_lock(&sys->spu_lock);
> > if (sys->spu)
> > + {
> > + vlc_mutex_lock(&sys->spu_lock);
> > spu_Destroy(sys->spu);
> > - vlc_mutex_unlock(&sys->spu_lock);
> > + vlc_mutex_unlock(&sys->spu_lock);
> > + }
> >
> > vout_Release(vout);
> > }
> > --
> > 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
More information about the vlc-devel
mailing list