[vlc-devel] [PATCH 4/5] vout: spu: move clock into channels

Thomas Guillem thomas at gllm.fr
Thu Jun 6 09:21:09 CEST 2019


On Thu, Jun 6, 2019, at 09:15, Roland Bewick wrote:
> Minor: I don't like the name RegisterSubpictureChannelAndClock. If you 
> have to do something else too would it be 
> RegisterSubpictureChannelAndClockAndSomethingElse()?
> 
> Maybe the old name was fine? you're just setting up a subpicture channel.

vlc_clock_t is not public. We must still let other modules register SPU channels withtout a clock, so I need to keep the original function untouched. I could rename it to RegisterSubpictureChannelInternal().

> 
> Roland
> 
> On 6/06/2019 1:28 PM, Thomas Guillem wrote:
> > This pave the way to multiple subtitles: each registered spu channel has now
> > its own clock.
> >
> > And fixes OSD subpictures date being messed up by the decoder clock.
> >
> > Fixes #22273
> > ---
> >   include/vlc_spu.h                    |  2 +-
> >   modules/stream_out/transcode/video.c |  2 +-
> >   src/input/decoder.c                  | 29 ++++++------
> >   src/video_output/video_output.c      | 47 +++++++++---------
> >   src/video_output/vout_internal.h     | 14 +++---
> >   src/video_output/vout_subpictures.c  | 71 +++++++++++++++++-----------
> >   6 files changed, 88 insertions(+), 77 deletions(-)
> >
> > diff --git a/include/vlc_spu.h b/include/vlc_spu.h
> > index 00ef08c71d..e4fb9a7ec1 100644
> > --- a/include/vlc_spu.h
> > +++ b/include/vlc_spu.h
> > @@ -74,7 +74,7 @@ VLC_API void spu_PutSubpicture( spu_t *, subpicture_t * );
> >    */
> >   VLC_API subpicture_t * spu_Render( spu_t *, const vlc_fourcc_t *p_chroma_list,
> >                                      const video_format_t *p_fmt_dst, const video_format_t *p_fmt_src,
> > -                                   vlc_tick_t system_now, vlc_tick_t pts, float rate,
> > +                                   vlc_tick_t system_now, vlc_tick_t pts,
> >                                      bool ignore_osd, bool external_scale );
> >   
> >   /**
> > diff --git a/modules/stream_out/transcode/video.c b/modules/stream_out/transcode/video.c
> > index ef64cf7539..9f1f72b8cf 100644
> > --- a/modules/stream_out/transcode/video.c
> > +++ b/modules/stream_out/transcode/video.c
> > @@ -378,7 +378,7 @@ static picture_t * RenderSubpictures( sout_stream_t *p_stream, sout_stream_id_sy
> >       }
> >   
> >       subpicture_t *p_subpic = spu_Render( id->p_spu, NULL, &fmt,
> > -                                         &outfmt, vlc_tick_now(), p_pic->date, 1.f,
> > +                                         &outfmt, vlc_tick_now(), p_pic->date,
> >                                            false, false );
> >   
> >       /* Overlay subpicture */
> > diff --git a/src/input/decoder.c b/src/input/decoder.c
> > index fde43681e4..580724944c 100644
> > --- a/src/input/decoder.c
> > +++ b/src/input/decoder.c
> > @@ -617,7 +617,6 @@ static subpicture_t *spu_new_buffer( decoder_t *p_dec,
> >                                                    p_owner->i_spu_channel);
> >                   p_owner->i_spu_channel = -1;
> >               }
> > -            vout_SetSubpictureClock(p_owner->p_vout, NULL);
> >   
> >               vlc_mutex_lock( &p_owner->lock );
> >               vout_Release(p_owner->p_vout);
> > @@ -630,25 +629,20 @@ static subpicture_t *spu_new_buffer( decoder_t *p_dec,
> >       if( p_owner->p_vout != p_vout )
> >       {
> >           ssize_t old_spu_channel = p_owner->i_spu_channel;
> > -        p_owner->i_spu_channel = vout_RegisterSubpictureChannel( p_vout );
> > +        p_owner->i_spu_channel =
> > +            vout_RegisterSubpictureChannelAndClock(p_vout, p_owner->p_clock);
> >           p_owner->i_spu_order = 0;
> >   
> >           if (p_owner->i_spu_channel != -1)
> >           {
> > -            if (p_owner->p_vout)
> > -            {
> > -                vout_SetSubpictureClock(p_owner->p_vout, NULL);
> > -                if (old_spu_channel != -1)
> > -                    vout_UnregisterSubpictureChannel(p_owner->p_vout,
> > -                                                     old_spu_channel);
> > -            }
> > +            if (p_owner->p_vout && old_spu_channel != -1)
> > +                vout_UnregisterSubpictureChannel(p_owner->p_vout,
> > +                                                 old_spu_channel);
> >               vlc_mutex_lock(&p_owner->lock);
> >               if (p_owner->p_vout)
> >                   vout_Release(p_owner->p_vout);
> >               p_owner->p_vout = p_vout;
> >               vlc_mutex_unlock(&p_owner->lock);
> > -
> > -            vout_SetSubpictureClock(p_vout, p_owner->p_clock);
> >           }
> >       }
> >       else
> > @@ -1548,7 +1542,11 @@ static void OutputChangeRate( decoder_t *p_dec, float rate )
> >               break;
> >           case SPU_ES:
> >               if( p_owner->p_vout != NULL )
> > -                vout_ChangeSpuRate( p_owner->p_vout, rate );
> > +            {
> > +                assert(p_owner->i_spu_channel != -1);
> > +                vout_ChangeSpuRate(p_owner->p_vout, p_owner->i_spu_channel,
> > +                                   rate );
> > +            }
> >               break;
> >           default:
> >               vlc_assert_unreachable();
> > @@ -1574,7 +1572,11 @@ static void OutputChangeDelay( decoder_t *p_dec, vlc_tick_t delay )
> >               break;
> >           case SPU_ES:
> >               if( p_owner->p_vout != NULL )
> > -                vout_ChangeSpuDelay( p_owner->p_vout, delay );
> > +            {
> > +                assert(p_owner->i_spu_channel != -1);
> > +                vout_ChangeSpuDelay(p_owner->p_vout, p_owner->i_spu_channel,
> > +                                    delay);
> > +            }
> >               break;
> >           default:
> >               vlc_assert_unreachable();
> > @@ -1991,7 +1993,6 @@ static void DeleteDecoder( decoder_t * p_dec )
> >                   assert( p_owner->i_spu_channel > 0 );
> >                   vout_UnregisterSubpictureChannel( p_owner->p_vout,
> >                                                     p_owner->i_spu_channel );
> > -                vout_SetSubpictureClock(p_owner->p_vout, NULL);
> >                   vout_Release(p_owner->p_vout);
> >               }
> >               break;
> > diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
> > index 1f0a2391dd..ae8dc8e515 100644
> > --- a/src/video_output/video_output.c
> > +++ b/src/video_output/video_output.c
> > @@ -280,20 +280,24 @@ ssize_t vout_RegisterSubpictureChannel( vout_thread_t *vout )
> >       return channel;
> >   }
> >   
> > -void vout_UnregisterSubpictureChannel( vout_thread_t *vout, size_t channel )
> > +ssize_t vout_RegisterSubpictureChannelAndClock(vout_thread_t *vout,
> > +                                               vlc_clock_t *clock)
> >   {
> >       assert(!vout->p->dummy);
> > +    ssize_t channel = VOUT_SPU_CHANNEL_INVALID;
> >   
> >       if (vout->p->spu)
> > -        spu_UnregisterChannel(vout->p->spu, channel);
> > +        channel = spu_RegisterChannelAndClock(vout->p->spu, clock);
> > +
> > +    return channel;
> >   }
> >   
> > -void vout_SetSubpictureClock( vout_thread_t *vout, vlc_clock_t *clock )
> > +void vout_UnregisterSubpictureChannel( vout_thread_t *vout, size_t channel )
> >   {
> >       assert(!vout->p->dummy);
> > +    assert(vout->p->spu);
> >       vlc_mutex_lock(&vout->p->spu_lock);
> > -    if (vout->p->spu)
> > -        spu_clock_Set(vout->p->spu, clock);
> > +    spu_UnregisterChannel(vout->p->spu, channel);
> >       vlc_mutex_unlock(&vout->p->spu_lock);
> >   }
> >   
> > @@ -301,10 +305,10 @@ void vout_FlushSubpictureChannel( vout_thread_t *vout, size_t channel )
> >   {
> >       vout_thread_sys_t *sys = vout->p;
> >       assert(!sys->dummy);
> > +    assert(sys->spu);
> >   
> >       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);
> >   }
> >   
> > @@ -1081,7 +1085,7 @@ static int ThreadDisplayRenderPicture(vout_thread_t *vout, bool is_forced)
> >       subpicture_t *subpic = spu_Render(sys->spu,
> >                                         subpicture_chromas, &fmt_spu_rot,
> >                                         &vd->source, system_now,
> > -                                      render_subtitle_date, sys->spu_rate,
> > +                                      render_subtitle_date,
> >                                         do_snapshot, vd->info.can_scale_spu);
> >       /*
> >        * Perform rendering
> > @@ -1313,14 +1317,6 @@ 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)
> > -    {
> > -        spu_clock_Reset(vout->p->spu);
> > -        spu_clock_SetDelay(vout->p->spu, vout->p->spu_delay);
> > -    }
> > -    vlc_mutex_unlock(&vout->p->spu_lock);
> >   }
> >   
> >   void vout_Flush(vout_thread_t *vout, vlc_tick_t date)
> > @@ -1378,21 +1374,22 @@ void vout_ChangeRate(vout_thread_t *vout, float rate)
> >       vout_control_Release(&sys->control);
> >   }
> >   
> > -void vout_ChangeSpuDelay(vout_thread_t *vout, vlc_tick_t delay)
> > +void vout_ChangeSpuDelay(vout_thread_t *vout, size_t channel_id,
> > +                         vlc_tick_t delay)
> >   {
> >       assert(!vout->p->dummy);
> > +    assert(vout->p->spu);
> >       vlc_mutex_lock(&vout->p->spu_lock);
> > -    if (vout->p->spu)
> > -        spu_clock_SetDelay(vout->p->spu, delay);
> > -    vout->p->spu_delay = delay;
> > +    spu_SetClockDelay(vout->p->spu, channel_id, delay);
> >       vlc_mutex_unlock(&vout->p->spu_lock);
> >   }
> >   
> > -void vout_ChangeSpuRate(vout_thread_t *vout, float rate)
> > +void vout_ChangeSpuRate(vout_thread_t *vout, size_t channel_id, float rate)
> >   {
> >       assert(!vout->p->dummy);
> > +    assert(vout->p->spu);
> >       vlc_mutex_lock(&vout->p->spu_lock);
> > -    vout->p->spu_rate = rate;
> > +    spu_SetClockRate(vout->p->spu, channel_id, rate);
> >       vlc_mutex_unlock(&vout->p->spu_lock);
> >   }
> >   
> > @@ -1946,10 +1943,10 @@ int vout_Request(const vout_configuration_t *cfg, input_thread_t *input)
> >       } else
> >           vout_UpdateWindowSizeLocked(vout);
> >   
> > -    sys->delay = sys->spu_delay = 0;
> > -    sys->rate = sys->spu_rate = 1.f;
> > +    sys->delay = 0;
> > +    sys->rate = 1.f;
> >       sys->clock = cfg->clock;
> > -    sys->delay = sys->spu_delay = 0;
> > +    sys->delay = 0;
> >   
> >       vlc_mutex_unlock(&sys->window_lock);
> >   
> > diff --git a/src/video_output/vout_internal.h b/src/video_output/vout_internal.h
> > index 56b4876f8b..d2cd60b4b7 100644
> > --- a/src/video_output/vout_internal.h
> > +++ b/src/video_output/vout_internal.h
> > @@ -72,9 +72,7 @@ struct vout_thread_sys_t
> >   
> >       vlc_clock_t     *clock;
> >       float           rate;
> > -    float           spu_rate;
> >       vlc_tick_t      delay;
> > -    vlc_tick_t      spu_delay;
> >   
> >       /* */
> >       video_format_t  original;   /* Original format ie coming from the decoder */
> > @@ -262,12 +260,12 @@ int vout_OpenWrapper(vout_thread_t *, const char *,
> >   void vout_CloseWrapper(vout_thread_t *);
> >   
> >   /* */
> > -void vout_SetSubpictureClock(vout_thread_t *vout, vlc_clock_t *clock);
> > +ssize_t vout_RegisterSubpictureChannelAndClock( vout_thread_t *, vlc_clock_t *clock );
> > +ssize_t spu_RegisterChannelAndClock( spu_t *, vlc_clock_t * );
> >   void spu_Attach( spu_t *, input_thread_t *input );
> >   void spu_Detach( spu_t * );
> > -void spu_clock_Set(spu_t *, vlc_clock_t *);
> > -void spu_clock_Reset(spu_t *);
> > -void spu_clock_SetDelay(spu_t *spu, vlc_tick_t delay);
> > +void spu_SetClockDelay(spu_t *spu, size_t channel_id, vlc_tick_t delay);
> > +void spu_SetClockRate(spu_t *spu, size_t channel_id, float rate);
> >   void spu_ChangeMargin(spu_t *, int);
> >   void spu_SetHighlight(spu_t *, const vlc_spu_highlight_t*);
> >   
> > @@ -293,12 +291,12 @@ void vout_ChangeDelay( vout_thread_t *, vlc_tick_t delay );
> >    * This function will change the rate of the spu channel
> >    * It is thread safe
> >    */
> > -void vout_ChangeSpuRate( vout_thread_t *, float rate );
> > +void vout_ChangeSpuRate( vout_thread_t *, size_t channel_id, float rate );
> >   /**
> >    * This function will change the delay of the spu channel
> >    * It is thread safe
> >    */
> > -void vout_ChangeSpuDelay( vout_thread_t *, vlc_tick_t delay );
> > +void vout_ChangeSpuDelay( vout_thread_t *, size_t channel_id, vlc_tick_t delay );
> >   
> >   
> >   /**
> > diff --git a/src/video_output/vout_subpictures.c b/src/video_output/vout_subpictures.c
> > index 17cc6a7b77..16902835dc 100644
> > --- a/src/video_output/vout_subpictures.c
> > +++ b/src/video_output/vout_subpictures.c
> > @@ -62,6 +62,9 @@ typedef struct {
> >   struct spu_channel {
> >       subpicture_t *entries[VOUT_MAX_SUBPICTURES];
> >       size_t id;
> > +    vlc_clock_t *clock;
> > +    vlc_tick_t delay;
> > +    float rate;
> >   };
> >   
> >   typedef struct VLC_VECTOR(struct spu_channel) spu_channel_vector;
> > @@ -71,7 +74,6 @@ struct spu_private_t {
> >       input_thread_t *input;
> >   
> >       spu_channel_vector channels;
> > -    vlc_clock_t *clock;
> >   
> >       int channel;             /**< number of subpicture channels registered */
> >       filter_t *text;                              /**< text renderer module */
> > @@ -102,9 +104,11 @@ struct spu_private_t {
> >       vout_thread_t       *vout;
> >   };
> >   
> > -static void spu_channel_Init(struct spu_channel *channel, size_t id)
> > +static void spu_channel_Init(struct spu_channel *channel, size_t id,
> > +                             vlc_clock_t *clock)
> >   {
> >       channel->id = id;
> > +    channel->clock = clock;
> >   
> >       for (size_t i = 0; i < VOUT_MAX_SUBPICTURES; i++)
> >           channel->entries[i] = NULL;
> > @@ -563,10 +567,8 @@ static int SpuRenderCmp(const void *s0, const void *s1)
> >   }
> >   
> >   static int spu_channel_ConvertDates(struct spu_channel *channel,
> > -                                    vlc_clock_t *clock,
> >                                       vlc_tick_t system_now,
> > -                                    spu_render_entry_t *render_entries,
> > -                                    float rate)
> > +                                    spu_render_entry_t *render_entries)
> >   {
> >       /* Put every spu start and stop ts into the same array to convert them in
> >        * one shot */
> > @@ -587,9 +589,9 @@ static int spu_channel_ConvertDates(struct spu_channel *channel,
> >           return 0;
> >   
> >       /* Convert all spu ts */
> > -    if (clock)
> > -        vlc_clock_ConvertArrayToSystem(clock, system_now, date_array,
> > -                                       entry_count * 2, rate);
> > +    if (channel->clock)
> > +        vlc_clock_ConvertArrayToSystem(channel->clock, system_now, date_array,
> > +                                       entry_count * 2, channel->rate);
> >   
> >       /* Put back the converted ts into the output spu_render_entry_t struct */
> >       entry_count = 0;
> > @@ -623,7 +625,7 @@ static int spu_channel_ConvertDates(struct spu_channel *channel,
> >    *****************************************************************************/
> >   static spu_render_entry_t *
> >   spu_SelectSubpictures(spu_t *spu, vlc_tick_t system_now,
> > -                      vlc_tick_t render_subtitle_date, float rate,
> > +                      vlc_tick_t render_subtitle_date,
> >                         bool ignore_osd, size_t *subpicture_count)
> >   {
> >       spu_private_t *sys = spu->p;
> > @@ -653,8 +655,7 @@ spu_SelectSubpictures(spu_t *spu, vlc_tick_t system_now,
> >           int64_t      ephemer_subtitle_order = INT64_MIN;
> >           int64_t      ephemer_system_order = INT64_MIN;
> >   
> > -        if (spu_channel_ConvertDates(channel, sys->clock, system_now, render_entries,
> > -                                     rate) == 0)
> > +        if (spu_channel_ConvertDates(channel, system_now, render_entries) == 0)
> >               continue;
> >   
> >           /* Select available pictures */
> > @@ -1405,15 +1406,13 @@ spu_t *spu_Create(vlc_object_t *object, vout_thread_t *vout)
> >       for (size_t i = 0; i < VOUT_SPU_CHANNEL_OSD_COUNT; ++i)
> >       {
> >           struct spu_channel channel;
> > -        spu_channel_Init(&channel, i);
> > +        spu_channel_Init(&channel, i, NULL);
> >           vlc_vector_push(&sys->channels, channel);
> >       }
> >   
> >       /* Initialize private fields */
> >       vlc_mutex_init(&sys->lock);
> >   
> > -    sys->clock = NULL;
> > -
> >       atomic_init(&sys->margin, var_InheritInteger(spu, "sub-margin"));
> >   
> >       sys->source_chain_update = NULL;
> > @@ -1522,21 +1521,27 @@ void spu_Detach(spu_t *spu)
> >       vlc_mutex_unlock(&spu->p->lock);
> >   }
> >   
> > -void spu_clock_Set(spu_t *spu, vlc_clock_t *clock)
> > +void spu_SetClockDelay(spu_t *spu, size_t channel_id, vlc_tick_t delay)
> >   {
> > -    spu->p->clock = clock;
> > -}
> > +    spu_private_t *sys = spu->p;
> >   
> > -void spu_clock_Reset(spu_t *spu)
> > -{
> > -    if (spu->p->clock)
> > -        vlc_clock_Reset(spu->p->clock);
> > +    vlc_mutex_lock(&sys->lock);
> > +    struct spu_channel *channel = spu_GetChannel(spu, channel_id);
> > +    assert(channel->clock);
> > +    vlc_clock_SetDelay(channel->clock, delay);
> > +    channel->delay = delay;
> > +    vlc_mutex_unlock(&sys->lock);
> >   }
> >   
> > -void spu_clock_SetDelay(spu_t *spu, vlc_tick_t delay)
> > +void spu_SetClockRate(spu_t *spu, size_t channel_id, float rate)
> >   {
> > -    if (spu->p->clock)
> > -        vlc_clock_SetDelay(spu->p->clock, delay);
> > +    spu_private_t *sys = spu->p;
> > +
> > +    vlc_mutex_lock(&sys->lock);
> > +    struct spu_channel *channel = spu_GetChannel(spu, channel_id);
> > +    assert(channel->clock);
> > +    channel->rate = rate;
> > +    vlc_mutex_unlock(&sys->lock);
> >   }
> >   
> >   /**
> > @@ -1639,7 +1644,7 @@ subpicture_t *spu_Render(spu_t *spu,
> >                            const video_format_t *fmt_dst,
> >                            const video_format_t *fmt_src,
> >                            vlc_tick_t system_now,
> > -                         vlc_tick_t render_subtitle_date, float rate,
> > +                         vlc_tick_t render_subtitle_date,
> >                            bool ignore_osd,
> >                            bool external_scale)
> >   {
> > @@ -1697,7 +1702,7 @@ subpicture_t *spu_Render(spu_t *spu,
> >   
> >       /* Get an array of subpictures to render */
> >       spu_render_entry_t *subpicture_array =
> > -        spu_SelectSubpictures(spu, system_now, render_subtitle_date, rate,
> > +        spu_SelectSubpictures(spu, system_now, render_subtitle_date,
> >                                ignore_osd, &subpicture_count);
> >       if (!subpicture_array)
> >       {
> > @@ -1747,7 +1752,7 @@ subpicture_t *spu_Render(spu_t *spu,
> >       return render;
> >   }
> >   
> > -ssize_t spu_RegisterChannel(spu_t *spu)
> > +ssize_t spu_RegisterChannelAndClock(spu_t *spu, vlc_clock_t *clock)
> >   {
> >       spu_private_t *sys = spu->p;
> >   
> > @@ -1758,7 +1763,7 @@ ssize_t spu_RegisterChannel(spu_t *spu)
> >       if (channel_id != VOUT_SPU_CHANNEL_INVALID)
> >       {
> >           struct spu_channel channel;
> > -        spu_channel_Init(&channel, channel_id);
> > +        spu_channel_Init(&channel, channel_id, clock);
> >           if (vlc_vector_push(&sys->channels, channel))
> >           {
> >               vlc_mutex_unlock(&sys->lock);
> > @@ -1771,6 +1776,11 @@ ssize_t spu_RegisterChannel(spu_t *spu)
> >       return VOUT_SPU_CHANNEL_INVALID;
> >   }
> >   
> > +ssize_t spu_RegisterChannel(spu_t *spu)
> > +{
> > +    return spu_RegisterChannelAndClock(spu, NULL);
> > +}
> > +
> >   static void spu_channel_Clear(struct spu_channel *channel)
> >   {
> >       for (size_t i = 0; i < VOUT_MAX_SUBPICTURES; i++)
> > @@ -1783,6 +1793,11 @@ void spu_ClearChannel(spu_t *spu, size_t channel_id)
> >       vlc_mutex_lock(&sys->lock);
> >       struct spu_channel *channel = spu_GetChannel(spu, channel_id);
> >       spu_channel_Clear(channel);
> > +    if (channel->clock)
> > +    {
> > +        vlc_clock_Reset(channel->clock);
> > +        vlc_clock_SetDelay(channel->clock, channel->delay);
> > +    }
> >       vlc_mutex_unlock(&sys->lock);
> >   }
> >   
> _______________________________________________
> 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