[vlc-devel] [PATCH 09/12] player: Save & restore playback states
Hugo Beauzée-Luyssen
hugo at beauzee.fr
Mon Aug 5 17:20:30 CEST 2019
On Wed, Jul 31, 2019, at 4:18 PM, Thomas Guillem wrote:
>
>
> On Wed, Jul 31, 2019, at 11:20, Hugo Beauzée-Luyssen wrote:
> > Refs #22524
> > ---
> > src/input/player.c | 171 +++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 171 insertions(+)
> >
> > diff --git a/src/input/player.c b/src/input/player.c
> > index e53849dc21..411456dfd8 100644
> > --- a/src/input/player.c
> > +++ b/src/input/player.c
> > @@ -35,6 +35,7 @@
> > #include <vlc_tick.h>
> > #include <vlc_decoder.h>
> > #include <vlc_memstream.h>
> > +#include <vlc_media_library.h>
> >
> > #include "libvlc.h"
> > #include "input_internal.h"
> > @@ -145,6 +146,8 @@ struct vlc_player_input
> > float pos;
> > bool set;
> > } abloop_state[2];
> > +
> > + vlc_ml_playback_states_all prefs;
> > };
> >
> > struct vlc_player_t
> > @@ -649,6 +652,43 @@ vlc_player_title_list_GetCount(struct
> > vlc_player_title_list *titles)
> > return titles->count;
> > }
> >
> > +static void
> > +vlc_player_input_restore_ml_prefs(struct vlc_player_input* input,
> > + const input_item_t* item)
>
> NIT: vlc_player_input_RestoreMlPrefs
>
Fixed
> > +{
> > + vlc_player_t* player = input->player;
> > + vlc_player_assert_locked(player);
> > +
> > + vlc_medialibrary_t* ml = vlc_ml_instance_get(input->player);
> > + if (!ml)
> > + return;
> > + vlc_ml_media_t* media = vlc_ml_get_media_by_mrl( ml,
> > item->psz_uri);
> > + if (!media)
> > + return;
> > + if (vlc_ml_media_get_all_playback_pref(ml, media->i_id,
> > &input->prefs) != VLC_SUCCESS)
> > + return;
> > + if (input->prefs.current_title >= 0)
> > + {
> > + /* If we are aiming at a specific title, wait for it to be
> > added, and
> > + * only then select it & set the position
> > + * If we're not aiming at a title, just set the position.
> > + */
> > + if (input->prefs.progress > .0f)
> > + input_SetPosition(input->thread, input->prefs.progress,
> > false);
>
> This doesn't seem to match the comment. YOu want to set position here
> only if current_title == -1, right ?
Indeed, and fixed
>
> > + }
> > + if (input->prefs.rate != .0f)
> > + vlc_player_ChangeRate(player, input->prefs.rate);
> > + if (input->prefs.aspect_ratio)
> > + var_SetString(input->thread, "aspect-ratio",
> > input->prefs.aspect_ratio);
> > + if (input->prefs.zoom)
> > + var_SetString(input->thread, "zoom", input->prefs.zoom);
> > + if (input->prefs.zoom)
> > + var_SetString(input->thread, "deinterlace", input->prefs.zoom);
> > + if (input->prefs.video_filter)
> > + var_SetString(input->thread, "video-filter",
> > input->prefs.video_filter);
>
> The vout is likely already created, so it won't inherit these variables.
>
Fixed
> > + vlc_ml_release(media);
> > +}
> > +
> > static struct vlc_player_input *
> > vlc_player_input_New(vlc_player_t *player, input_item_t *item)
> > {
> > @@ -687,6 +727,14 @@ vlc_player_input_New(vlc_player_t *player,
> > input_item_t *item)
> >
> > input->abloop_state[0].set = input->abloop_state[1].set = false;
> >
> > + memset(&input->prefs, 0, sizeof(input->prefs));
> > + input->prefs.aspect_ratio = input->prefs.zoom = input->prefs.crop =
> > + input->prefs.deinterlace = input->prefs.video_filter = NULL;
> > + input->prefs.current_title = input->prefs.current_video_track =
> > + input->prefs.current_audio_track =
> > + input->prefs.current_subtitle_track = -1;
> > + input->prefs.progress = -1.f;
> > +
> > input->thread = input_Create(player, input_thread_Events, input,
> > item,
> > player->resource, player->renderer);
> > if (!input->thread)
> > @@ -695,6 +743,8 @@ vlc_player_input_New(vlc_player_t *player,
> > input_item_t *item)
> > return NULL;
> > }
> >
> > + vlc_player_input_restore_ml_prefs(input, item);
> > +
> > /* Initial sub/audio delay */
> > const vlc_tick_t cat_delays[DATA_ES] = {
> > [AUDIO_ES] =
> > @@ -892,6 +942,87 @@ static int vlc_player_selected_track( const
> > vlc_player_track_vector* tracks )
> > return 0;
> > }
> >
> > +static void vlc_player_UpdateML(vlc_player_t *player,
> > + struct vlc_player_input* input)
> > +{
> > + vlc_medialibrary_t* ml = vlc_ml_instance_get(player);
> > + if (!ml)
> > + return;
> > + input_item_t* item = input_GetItem(input->thread);
> > + if (!item)
> > + return;
> > + vlc_ml_media_t* media = vlc_ml_get_media_by_mrl(ml, item->psz_uri);
> > + if (!media)
> > + {
> > + /* We don't know this media yet, let's add it as an external
> > media so
> > + * we can still store its playback preferences
> > + */
> > + media = vlc_ml_new_external_media(ml, item->psz_uri);
> > + if (media == NULL)
> > + return;
> > + }
> > +
> > + input->prefs.progress = input->position;
> > + input->prefs.current_title = input->title_selected;
> > + if (input->rate != input->prefs.rate)
> > + input->prefs.rate = input->rate;
> > + else
> > + input->prefs.rate = -1.f;
> > +
> > +#define COMPARE_ASSIGN_STR(field, var) \
>
> You could add a little comment to explain what this macro is doing
>
Done
> > + char* field = var_GetNonEmptyString(player, var); \
> > + if ( ( field != NULL && input->prefs.field != NULL &&
> > strcmp(field, input->prefs.field)) || \
> > + ( field == NULL && input->prefs.field != NULL ) || \
> > + ( field != NULL && input->prefs.field == NULL ) ) \
> > + { \
> > + free(input->prefs.field); \
> > + input->prefs.field = field; \
> > + field = NULL; \
> > + } \
> > + else \
> > + { \
> > + free(input->prefs.field); \
> > + input->prefs.field = NULL; \
> > + }
> > +
> > + COMPARE_ASSIGN_STR(aspect_ratio, "aspect-ratio" );
> > + COMPARE_ASSIGN_STR(zoom, "zoom");
> > + COMPARE_ASSIGN_STR(crop, "crop");
> > + COMPARE_ASSIGN_STR(deinterlace, "deinterlace");
> > + COMPARE_ASSIGN_STR(video_filter, "video-filter");
>
> These variables could be on the player, yes but they won't be up to
> date. Player users have to change these variables on vouts. So maybe
> you should query the vout, but the good one, cf. vlc_player_GetEsIdVout
>
Done
> > +
> > +#undef COMPARE_ASSIGN_STR
> > +
> > + int current_video_track =
> > vlc_player_selected_track(&input->video_track_vector);
> > + int current_audio_track =
> > vlc_player_selected_track(&input->audio_track_vector);
> > + int current_subtitle_track =
> > vlc_player_selected_track(&input->spu_track_vector);
> > +
> > + if (input->prefs.current_video_track != current_video_track)
> > + input->prefs.current_video_track = current_video_track;
> > + else
> > + input->prefs.current_video_track = -1;
>
> I don't understand why you would set it to -1 if tracks are equal.
>
If the value didn't change, we don't want to update it and to store it again (whether it's the default value or a state that was previously updated but hasn't changed during this playback), so we set it to -1 (for these states) which means that they won't be saved in the media library.
> > +
> > + if (input->prefs.current_audio_track != current_audio_track)
> > + input->prefs.current_audio_track = current_audio_track;
> > + else
> > + input->prefs.current_audio_track = -1;
> > +
> > + if (input->prefs.current_subtitle_track != current_subtitle_track)
> > + input->prefs.current_subtitle_track = current_subtitle_track;
> > + else
> > + input->prefs.current_subtitle_track = -1;
> > +
> > + vlc_ml_media_set_all_playback_states(ml, media->i_id,
> > &input->prefs);
> > + vlc_ml_release(&input->prefs);
> > + vlc_ml_release(media);
> > +
> > + free(video_filter);
> > + free(deinterlace);
> > + free(crop);
> > + free(zoom);
> > + free(aspect_ratio);
> > +}
> > +
> > static void *
> > vlc_player_destructor_Thread(void *data)
> > {
> > @@ -916,6 +1047,7 @@ vlc_player_destructor_Thread(void *data)
> > vlc_player_input_HandleState(input,
> > VLC_PLAYER_STATE_STOPPING);
> > vlc_player_destructor_AddStoppingInput(player, input);
> >
> > + vlc_player_UpdateML(player, input);
> > input_Stop(input->thread);
> > }
> >
> > @@ -1840,6 +1972,28 @@ vlc_player_input_HandleEsEvent(struct
> > vlc_player_input *input,
> > }
> > vlc_player_SendEvent(player, on_track_list_changed,
> > VLC_PLAYER_LIST_ADDED, &trackpriv->t);
> > + switch(ev->fmt->i_cat)
> > + {
> > + case VIDEO_ES:
> > + if (input->prefs.current_video_track != -1 &&
> > + input->prefs.current_video_track ==
> > ev->fmt->i_id)
> > + vlc_player_SelectTrack(input->player,
> > &trackpriv->t,
> > +
> > VLC_PLAYER_SELECT_EXCLUSIVE);
> > + break;
> > + case AUDIO_ES:
> > + if (input->prefs.current_audio_track != -1 &&
> > + input->prefs.current_audio_track ==
> > ev->fmt->i_id)
> > + vlc_player_SelectTrack(input->player,
> > &trackpriv->t,
> > +
> > VLC_PLAYER_SELECT_EXCLUSIVE);
> > + break;
> > + case SPU_ES:
> > + if (input->prefs.current_subtitle_track != -1 &&
> > + input->prefs.current_subtitle_track ==
> > ev->fmt->i_id)
> > + vlc_player_SelectTrack(input->player,
> > &trackpriv->t,
> > +
> > VLC_PLAYER_SELECT_EXCLUSIVE);
> > + default:
> > + break;
> > + }
> > break;
> > case VLC_INPUT_ES_DELETED:
> > {
> > @@ -1907,8 +2061,15 @@ vlc_player_input_HandleTitleEvent(struct
> > vlc_player_input *input,
> > title_offset,
> > chapter_offset);
> > vlc_player_SendEvent(player, on_titles_changed,
> > input->titles);
> > if (input->titles)
> > + {
> > vlc_player_SendEvent(player,
> > on_title_selection_changed,
> > &input->titles->array[0], 0);
> > + if (input->prefs.current_title >= 0 &&
> > + (size_t)input->prefs.current_title <
> > ev->list.count)
> > + {
> > + vlc_player_SelectTitleIdx(player,
> > input->prefs.current_title);
> > + }
> > + }
> > break;
> > }
> > case VLC_INPUT_TITLE_SELECTED:
> > @@ -1919,6 +2080,16 @@ vlc_player_input_HandleTitleEvent(struct
> > vlc_player_input *input,
> > vlc_player_SendEvent(player, on_title_selection_changed,
> >
> > &input->titles->array[input->title_selected],
> > input->title_selected);
> > + if (input->prefs.current_title >= 0 &&
> > + (size_t)input->prefs.current_title == ev->selected_idx
> > &&
> > + input->prefs.progress > .0f)
> > + {
> > + input_SetPosition(input->thread,
> > input->prefs.progress, false);
> > + /* Reset the wanted title to avoid forcing it or the
> > position
> > + * again during the next title change
> > + */
> > + input->prefs.current_title = 0;
> > + }
> > break;
> > default:
> > vlc_assert_unreachable();
> > --
>
> There can be several players instances. I think only the main one,
> owned by the main playlist should play with the medialibrary.
Done
A new version should arrive shortly.
Thanks,
--
Hugo Beauzée-Luyssen
hugo at beauzee.fr
More information about the vlc-devel
mailing list