[vlc-devel] [PATCH 4/5] player: merge Set/Get Audio/Subtitle delay into Set/Get Category delay

Thomas Guillem thomas at gllm.fr
Tue Jun 18 17:54:45 CEST 2019



On Tue, Jun 18, 2019, at 17:38, Roland Bewick wrote:
> 
> On 18/06/2019 10:28 PM, Thomas Guillem wrote:
> > ---
> >   include/vlc_player.h                          | 90 +++++++++--------
> >   .../gui/macosx/playlist/VLCPlayerController.m | 27 ++---
> >   .../gui/qt/components/player_controller.cpp   | 35 ++++---
> >   src/input/player.c                            | 98 +++++++++----------
> >   src/libvlccore.sym                            |  5 +-
> >   test/src/input/player.c                       | 15 +--
> >   6 files changed, 130 insertions(+), 140 deletions(-)
> >
> > diff --git a/include/vlc_player.h b/include/vlc_player.h
> > index 403f405d52..3882ae29cf 100644
> > --- a/include/vlc_player.h
> > +++ b/include/vlc_player.h
> > @@ -281,8 +281,7 @@ enum vlc_player_seek_speed
> >    *
> >    * @see vlc_player_SeekByPos()
> >    * @see vlc_player_SeekByTime()
> > - * @see vlc_player_SetAudioDelay()
> > - * @see vlc_player_SetSubtitleDelay()
> > + * @see vlc_player_SetCategoryDelay()
> >    */
> >   enum vlc_player_whence
> >   {
> > @@ -671,28 +670,17 @@ struct vlc_player_cbs
> >           bool enabled, void *data);
> >   
> >       /**
> > -     * Called when the player audio delay has changed
> > +     * Called when the player category delay has changed
> >        *
> > -     * @see vlc_player_SetAudioDelay()
> > +     * @see vlc_player_SetCategoryDelay()
> >        *
> >        * @param player locked player instance
> > +     * @param cat AUDIO_ES or SPU_ES
> >        * @param new_delay audio delay
> >        * @param data opaque pointer set by vlc_player_AddListener()
> >        */
> > -    void (*on_audio_delay_changed)(vlc_player_t *player,
> > -        vlc_tick_t new_delay, void *data);
> > -
> > -    /**
> > -     * Called when the player subtitle delay has changed
> > -     *
> > -     * @see vlc_player_SetSubtitleDelay()
> > -     *
> > -     * @param player locked player instance
> > -     * @param new_delay subtitle delay
> > -     * @param data opaque pointer set by vlc_player_AddListener()
> > -     */
> > -    void (*on_subtitle_delay_changed)(vlc_player_t *player,
> > -        vlc_tick_t new_delay, void *data);
> > +    void (*on_category_delay_changed)(vlc_player_t *player,
> > +         enum es_format_category_e cat, vlc_tick_t new_delay, void *data);
> >   
> >       /**
> >        * Called when associated subtitle has changed
> > @@ -2404,52 +2392,70 @@ vlc_player_ToggleRecording(vlc_player_t *player)
> >   }
> >   
> >   /**
> > - * Get the audio delay for the current media
> > + * Get the delay of a category
> >    *
> > - * @see vlc_player_cbs.on_audio_delay_changed
> > + * @see vlc_player_cbs.on_category_delay_changed
> >    *
> >    * @param player locked player instance
> > + * @param cat AUDIO_ES or SPU_ES (VIDEO_ES not supported yet)
> >    */
> >   VLC_API vlc_tick_t
> > -vlc_player_GetAudioDelay(vlc_player_t *player);
> > +vlc_player_GetCategoryDelay(vlc_player_t *player, enum es_format_category_e cat);
> 
> With dual track support coming, how will you get the secondary subtitle 
> delay using this function? will you add an additional parameter here?

I plan to use new functions:

vlc_player_SetTrackDelay(vlc_player_t *, vlc_es_id_t *es_id, vlc_tick_t delay, whence);
vlc_player_GetTrackDelay(vlc_player_t *, vlc_es_id_t *es_id, vlc_tick_t delay, whence);

Calling vlc_player_SetTrackDelay() will overide delay previously set for that category and vice-versa.

Users will still be able to setup delay without taking care of tracks with vlc_player_SetCategoryDelay(). 

By the way, here is my WIP branch https://code.videolan.org/tguillem/vlc/commits/multiple-es/2 that is based on your work.
It's not ready to be sent on the ML. I started from your commits, but did some modifications (I let your authorship). Do you agree with that ? As you can see, this branch is not complete and miss your last commits.

> 
> >   
> >   /**
> > - * Set the audio delay for the current media
> > + * Set the delay of one category for the current media
> >    *
> >    * @note A successful call will trigger the
> > - * vlc_player_cbs.on_audio_delay_changed event.
> > + * vlc_player_cbs.on_category_delay_changed event.
> >    *
> >    * @param player locked player instance
> > + * @param cat AUDIO_ES or SPU_ES (VIDEO_ES not supported yet)
> >    * @param delay a valid time
> >    * @param whence absolute or relative
> > + * @return VLC_SUCCESS or VLC_EGENERIC if the category is not handled
> >    */
> > -VLC_API void
> > +VLC_API int
> > +vlc_player_SetCategoryDelay(vlc_player_t *player, enum es_format_category_e cat,
> > +                            vlc_tick_t delay, enum vlc_player_whence whence);
> > +
> > +/**
> > + * Helper to get the audio delay
> > + */
> > +static inline vlc_tick_t
> > +vlc_player_GetAudioDelay(vlc_player_t *player)
> > +{
> > +    return vlc_player_GetCategoryDelay(player, AUDIO_ES);
> > +}
> > +
> > +/**
> > + * Helper to set the audio delay
> > + */
> > +static inline void
> >   vlc_player_SetAudioDelay(vlc_player_t *player, vlc_tick_t delay,
> > -                         enum vlc_player_whence whence);
> > +                         enum vlc_player_whence whence)
> > +
> > +{
> > +    vlc_player_SetCategoryDelay(player, AUDIO_ES, delay, whence);
> > +}
> >   
> >   /**
> > - * Get the subtitle delay for the current media
> > - *
> > - * @see vlc_player_cbs.on_subtitle_delay_changed
> > - *
> > - * @param player locked player instance
> > + * Helper to get the subtitle delay
> >    */
> > -VLC_API vlc_tick_t
> > -vlc_player_GetSubtitleDelay(vlc_player_t *player);
> > +static inline vlc_tick_t
> > +vlc_player_GetSubtitleDelay(vlc_player_t *player)
> > +{
> > +    return vlc_player_GetCategoryDelay(player, SPU_ES);
> > +}
> >   
> >   /**
> > - * Set the subtitle delay for the current media
> > - *
> > - * @note A successful call will trigger the
> > - * vlc_player_cbs.on_subtitle_delay_changed event.
> > - *
> > - * @param player locked player instance
> > - * @param delay a valid time
> > - * @param whence absolute or relative
> > + * Helper to set the subtitle delay
> >    */
> > -VLC_API void
> > +static inline void
> >   vlc_player_SetSubtitleDelay(vlc_player_t *player, vlc_tick_t delay,
> > -                            enum vlc_player_whence whence);
> > +                            enum vlc_player_whence whence)
> > +{
> > +    vlc_player_SetCategoryDelay(player, SPU_ES, delay, whence);
> > +}
> >   
> >   /**
> >    * Set the subtitle text scaling factor
> > diff --git a/modules/gui/macosx/playlist/VLCPlayerController.m b/modules/gui/macosx/playlist/VLCPlayerController.m
> > index 04f71258d0..a3478417ae 100644
> > --- a/modules/gui/macosx/playlist/VLCPlayerController.m
> > +++ b/modules/gui/macosx/playlist/VLCPlayerController.m
> > @@ -284,21 +284,23 @@ static void cb_player_teletext_transparency_changed(vlc_player_t *p_player, bool
> >       });
> >   }
> >   
> > -static void cb_player_audio_delay_changed(vlc_player_t *p_player, vlc_tick_t newDelay, void *p_data)
> > +static void cb_player_category_delay_changed(vlc_player_t *p_player, enum es_format_category_e cat,
> > +                                             vlc_tick_t newDelay, void *p_data)
> >   {
> >       VLC_UNUSED(p_player);
> >       dispatch_async(dispatch_get_main_queue(), ^{
> >           VLCPlayerController *playerController = (__bridge VLCPlayerController *)p_data;
> > -        [playerController audioDelayChanged:newDelay];
> > -    });
> > -}
> > -
> > -static void cb_player_subtitle_delay_changed(vlc_player_t *p_player, vlc_tick_t newDelay, void *p_data)
> > -{
> > -    VLC_UNUSED(p_player);
> > -    dispatch_async(dispatch_get_main_queue(), ^{
> > -        VLCPlayerController *playerController = (__bridge VLCPlayerController *)p_data;
> > -        [playerController subtitlesDelayChanged:newDelay];
> > +        switch (cat)
> > +        {
> > +            case AUDIO_ES:
> > +                [playerController audioDelayChanged:newDelay];
> > +                break;
> > +            case SPU_ES:
> > +                [playerController subtitlesDelayChanged:newDelay];
> > +                break;
> > +            default:
> > +                vlc_assert_unreachable();
> > +        }
> >       });
> >   }
> >   
> > @@ -464,8 +466,7 @@ static const struct vlc_player_cbs player_callbacks = {
> >       cb_player_teletext_enabled_changed,
> >       cb_player_teletext_page_changed,
> >       cb_player_teletext_transparency_changed,
> > -    cb_player_audio_delay_changed,
> > -    cb_player_subtitle_delay_changed,
> > +    cb_player_category_delay_changed,
> >       cb_player_associated_subs_fps_changed,
> >       cb_player_renderer_changed,
> >       cb_player_record_changed,
> > diff --git a/modules/gui/qt/components/player_controller.cpp b/modules/gui/qt/components/player_controller.cpp
> > index 3e386daf69..aeb5954cfa 100644
> > --- a/modules/gui/qt/components/player_controller.cpp
> > +++ b/modules/gui/qt/components/player_controller.cpp
> > @@ -579,25 +579,25 @@ static void on_player_teletext_transparency_changed(vlc_player_t *, bool enabled
> >       });
> >   }
> >   
> > -static void on_player_audio_delay_changed(vlc_player_t *, vlc_tick_t new_delay,
> > +static void on_player_category_delay_changed(vlc_player_t *,
> > +                               enum es_format_category_e cat, vlc_tick_t new_delay,
> >                                  void *data)
> >   {
> >       PlayerControllerPrivate* that = static_cast<PlayerControllerPrivate*>(data);
> > -    msg_Dbg( that->p_intf, "on_player_audio_delay_changed");
> > -    that->callAsync([that,new_delay] (){
> > -        that->m_audioDelay = new_delay;
> > -        emit that->q_func()->audioDelayChanged( new_delay );
> > -    });
> > -}
> > -
> > -static void on_player_subtitle_delay_changed(vlc_player_t *, vlc_tick_t new_delay,
> > -                                  void *data)
> > -{
> > -    PlayerControllerPrivate* that = static_cast<PlayerControllerPrivate*>(data);
> > -    msg_Dbg( that->p_intf, "on_player_subtitle_delay_changed");
> > -    that->callAsync([that,new_delay] (){
> > -        that->m_subtitleDelay = new_delay;
> > -        emit that->q_func()->subtitleDelayChanged( new_delay );
> > +    msg_Dbg( that->p_intf, "on_player_category_delay_changed: %d", cat );
> > +    that->callAsync([that,cat,new_delay] (){
> > +        switch (cat)
> > +        {
> > +            case AUDIO_ES:
> > +                that->m_audioDelay = new_delay;
> > +                emit that->q_func()->audioDelayChanged( new_delay );
> > +                break;
> > +            case SPU_ES:
> > +                that->m_subtitleDelay = new_delay;
> > +                emit that->q_func()->subtitleDelayChanged( new_delay );
> > +                break;
> > +            default: vlc_assert_unreachable();
> > +        }
> >       });
> >   }
> >   
> > @@ -819,8 +819,7 @@ static const struct vlc_player_cbs player_cbs = {
> >       on_player_teletext_enabled_changed,
> >       on_player_teletext_page_changed,
> >       on_player_teletext_transparency_changed,
> > -    on_player_audio_delay_changed,
> > -    on_player_subtitle_delay_changed,
> > +    on_player_category_delay_changed,
> >       on_player_associated_subs_fps_changed,
> >       on_player_renderer_changed,
> >       on_player_record_changed,
> > diff --git a/src/input/player.c b/src/input/player.c
> > index cdb170f167..b5f8a8f46c 100644
> > --- a/src/input/player.c
> > +++ b/src/input/player.c
> > @@ -55,6 +55,7 @@ struct vlc_player_track_priv
> >   {
> >       struct vlc_player_track t;
> >       vout_thread_t *vout; /* weak reference */
> > +    vlc_tick_t delay;
> >   };
> >   
> >   typedef struct VLC_VECTOR(struct vlc_player_program *)
> > @@ -417,6 +418,7 @@ vlc_player_track_New(vlc_es_id_t *id, const char *name, const es_format_t *fmt)
> >       struct vlc_player_track_priv *trackpriv = malloc(sizeof(*trackpriv));
> >       if (!trackpriv)
> >           return NULL;
> > +    trackpriv->delay = 0;
> >       struct vlc_player_track *track = &trackpriv->t;
> >       track->name = strdup(name);
> >       if (!track->name)
> > @@ -2053,12 +2055,12 @@ input_thread_Events(input_thread_t *input_thread,
> >               break;
> >           case INPUT_EVENT_AUDIO_DELAY:
> >               input->audio_delay = event->audio_delay;
> > -            vlc_player_SendEvent(player, on_audio_delay_changed,
> > +            vlc_player_SendEvent(player, on_category_delay_changed, AUDIO_ES,
> >                                    input->audio_delay);
> >               break;
> >           case INPUT_EVENT_SUBTITLE_DELAY:
> >               input->subtitle_delay = event->subtitle_delay;
> > -            vlc_player_SendEvent(player, on_subtitle_delay_changed,
> > +            vlc_player_SendEvent(player, on_category_delay_changed, SPU_ES,
> >                                    input->subtitle_delay);
> >               break;
> >           case INPUT_EVENT_CACHE:
> > @@ -2823,62 +2825,61 @@ vlc_player_SetRecordingEnabled(vlc_player_t *player, bool enable)
> >                                  _("Recording") : _("Recording done"));
> >   }
> >   
> > -void
> > -vlc_player_SetAudioDelay(vlc_player_t *player, vlc_tick_t delay,
> > -                         enum vlc_player_whence whence)
> > +int
> > +vlc_player_SetCategoryDelay(vlc_player_t *player, enum es_format_category_e cat,
> > +                         vlc_tick_t delay, enum vlc_player_whence whence)
> >   {
> >       bool absolute = whence == VLC_PLAYER_WHENCE_ABSOLUTE;
> >       struct vlc_player_input *input = vlc_player_get_input_locked(player);
> >       if (!input)
> > -        return;
> > +        return VLC_EGENERIC;
> >   
> > -    input_ControlPush(input->thread, INPUT_CONTROL_SET_AUDIO_DELAY,
> > -        &(input_control_param_t) {
> > -            .delay = {
> > -                .b_absolute = whence == VLC_PLAYER_WHENCE_ABSOLUTE,
> > -                .i_val = delay,
> > -            },
> > -    });
> > +    switch (cat)
> > +    {
> > +        case AUDIO_ES:
> > +            input_ControlPush(input->thread, INPUT_CONTROL_SET_AUDIO_DELAY,
> > +                &(input_control_param_t) {
> > +                    .delay = {
> > +                        .b_absolute = absolute,
> > +                        .i_val = delay,
> > +                    },
> > +            });
> > +            break;
> > +        case SPU_ES:
> > +            input_ControlPush(input->thread, INPUT_CONTROL_SET_SPU_DELAY,
> > +                &(input_control_param_t) {
> > +                    .delay = {
> > +                        .b_absolute = absolute,
> > +                        .i_val = delay,
> > +                    },
> > +            });
> > +            break;
> > +        default:
> > +            return VLC_EGENERIC;
> > +    }
> >   
> > -    if (!absolute)
> > -        delay += input->audio_delay;
> > -    vlc_player_vout_OSDMessage(player, _("Audio delay: %i ms"),
> > +    vlc_player_vout_OSDMessage(player, _("%s delay: %s%i ms"),
> > +                               es_format_category_to_string(cat),
> > +                               absolute ? "" : "+",
> >                                  (int)MS_FROM_VLC_TICK(delay));
> > +    return VLC_SUCCESS;
> >   }
> >   
> >   vlc_tick_t
> > -vlc_player_GetAudioDelay(vlc_player_t *player)
> > +vlc_player_GetCategoryDelay(vlc_player_t *player, enum es_format_category_e cat)
> >   {
> > -    struct vlc_player_input *input = vlc_player_get_input_locked(player);
> > -    return input ? input->audio_delay : 0;
> > -}
> > -
> > -static void
> > -vlc_player_SetSubtitleDelayInternal(vlc_player_t *player, vlc_tick_t delay,
> > -                                    enum vlc_player_whence whence)
> > -{
> > -    bool absolute = whence == VLC_PLAYER_WHENCE_ABSOLUTE;
> >       struct vlc_player_input *input = vlc_player_get_input_locked(player);
> >       if (!input)
> > -        return;
> > -
> > -    input_ControlPush(input->thread, INPUT_CONTROL_SET_SPU_DELAY,
> > -        &(input_control_param_t) {
> > -            .delay = {
> > -                .b_absolute = absolute,
> > -                .i_val = delay,
> > -            },
> > -    });
> > -}
> > -
> > -void
> > -vlc_player_SetSubtitleDelay(vlc_player_t *player, vlc_tick_t delay,
> > -                            enum vlc_player_whence whence)
> > -{
> > -    vlc_player_SetSubtitleDelayInternal(player, delay, whence);
> > -    vlc_player_vout_OSDMessage(player, _("Subtitle delay: %s%i ms"),
> > -                               whence == VLC_PLAYER_WHENCE_ABSOLUTE ? "" : "+",
> > -                               (int)MS_FROM_VLC_TICK(delay));
> > +        return 0;
> > +    switch (cat)
> > +    {
> > +        case AUDIO_ES:
> > +            return input->audio_delay;
> > +        case SPU_ES:
> > +            return input->subtitle_delay;
> > +        default:
> > +            return 0;
> > +    }
> >   }
> >   
> >   static struct {
> > @@ -2931,13 +2932,6 @@ vlc_player_GetSubtitleTextScale(vlc_player_t *player)
> >       return var_GetInteger(player, "sub-text-scale");
> >   }
> >   
> > -vlc_tick_t
> > -vlc_player_GetSubtitleDelay(vlc_player_t *player)
> > -{
> > -    struct vlc_player_input *input = vlc_player_get_input_locked(player);
> > -    return input ? input->subtitle_delay : 0;
> > -}
> > -
> >   int
> >   vlc_player_GetSignal(vlc_player_t *player, float *quality, float *strength)
> >   {
> > diff --git a/src/libvlccore.sym b/src/libvlccore.sym
> > index 8cca2bbc01..6a4cc6040c 100644
> > --- a/src/libvlccore.sym
> > +++ b/src/libvlccore.sym
> > @@ -783,8 +783,8 @@ vlc_player_CondWait
> >   vlc_player_DisplayPosition
> >   vlc_player_DecrementRate
> >   vlc_player_Delete
> > -vlc_player_GetAudioDelay
> >   vlc_player_GetCapabilities
> > +vlc_player_GetCategoryDelay
> >   vlc_player_GetCategoryLanguage
> >   vlc_player_GetCurrentMedia
> >   vlc_player_GetError
> > @@ -801,7 +801,6 @@ vlc_player_GetSelectedTitleIdx
> >   vlc_player_GetSignal
> >   vlc_player_GetState
> >   vlc_player_GetStatistics
> > -vlc_player_GetSubtitleDelay
> >   vlc_player_GetSubtitleTextScale
> >   vlc_player_GetTeletextPage
> >   vlc_player_GetTime
> > @@ -848,7 +847,7 @@ vlc_player_SelectTitleIdx
> >   vlc_player_SelectTrack
> >   vlc_player_SetAssociatedSubsFPS
> >   vlc_player_SetAtoBLoop
> > -vlc_player_SetAudioDelay
> > +vlc_player_SetCategoryDelay
> >   vlc_player_SetCurrentMedia
> >   vlc_player_SetMediaStoppedAction
> >   vlc_player_SetRecordingEnabled
> > diff --git a/test/src/input/player.c b/test/src/input/player.c
> > index 2c031f83a4..e467f60073 100644
> > --- a/test/src/input/player.c
> > +++ b/test/src/input/player.c
> > @@ -104,8 +104,7 @@ struct report_media_subitems
> >       X(vlc_player_title_list *, on_titles_changed) \
> >       X(size_t, on_title_selection_changed) \
> >       X(struct report_chapter_selection, on_chapter_selection_changed) \
> > -    X(vlc_tick_t, on_audio_delay_changed) \
> > -    X(vlc_tick_t, on_subtitle_delay_changed) \
> > +    X(vlc_tick_t, on_category_delay_changed) \
> >       X(bool, on_recording_changed) \
> >       X(struct report_signal, on_signal_changed) \
> >       X(struct input_stats_t, on_statistics_changed) \
> > @@ -389,19 +388,11 @@ player_on_chapter_selection_changed(vlc_player_t *player,
> >   }
> >   
> >   static void
> > -player_on_audio_delay_changed(vlc_player_t *player, vlc_tick_t new_delay,
> > +player_on_category_delay_changed(vlc_player_t *player, vlc_tick_t new_delay,
> >                                 void *data)
> >   {
> >       struct ctx *ctx = get_ctx(player, data);
> > -    VEC_PUSH(on_audio_delay_changed, new_delay);
> > -}
> > -
> > -static void
> > -player_on_subtitle_delay_changed(vlc_player_t *player, vlc_tick_t new_delay,
> > -                                 void *data)
> > -{
> > -    struct ctx *ctx = get_ctx(player, data);
> > -    VEC_PUSH(on_subtitle_delay_changed, new_delay);
> > +    VEC_PUSH(on_category_delay_changed, new_delay);
> >   }
> >   
> >   static void
> _______________________________________________
> 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