[vlc-devel] [PATCH] player: fix teletext handling
Thomas Guillem
thomas at gllm.fr
Thu May 28 13:15:33 CEST 2020
Hello,
On Wed, May 27, 2020, at 14:33, Francois Cartegnie wrote:
> reworked version of the earlier teletext patches
> to handle index pages declared as subtitles.
>
> Also now properly trigger, including missing, events.
>
> ---
> src/player/input.c | 127 +++++++++++++++++++++++++++++++++-----------
> src/player/player.c | 19 ++++---
> src/player/player.h | 2 +-
> 3 files changed, 106 insertions(+), 42 deletions(-)
>
> diff --git a/src/player/input.c b/src/player/input.c
> index 093a8fcc99..092cd6976b 100644
> --- a/src/player/input.c
> +++ b/src/player/input.c
> @@ -354,48 +354,116 @@ vlc_player_input_HandleProgramEvent(struct
> vlc_player_input *input,
> }
> }
>
> +static const struct vlc_player_track_priv *
> +vlc_player_FindTeletextSource(const struct vlc_player_input *input,
> + const struct vlc_player_track_priv
> *exclude,
> + bool selected)
> +{
> + const struct vlc_player_track_priv *t;
> + vlc_vector_foreach(t, &input->spu_track_vector)
> + {
> + if(t->t.fmt.i_codec == VLC_CODEC_TELETEXT &&
Nit: could you respect the coding style of the file ? "if ("
> + t != exclude &&
> + t->t.selected == selected)
> + return t;
> + }
> + return NULL;
> +}
> +
> +static unsigned
> +vlc_player_input_TeletextUserPage(const struct vlc_player_track_priv
> *t)
> +{
> + const uint8_t mag = t->t.fmt.subs.teletext.i_magazine;
> + const uint8_t page = t->t.fmt.subs.teletext.i_page;
> + return (mag % 8) * 100 +
> + (page & 15) + ((page >> 4) & 15) * 10 + ((page >> 8) & 15)
> * 100;
> +}
> +
> static void
> vlc_player_input_HandleTeletextMenu(struct vlc_player_input *input,
> - const struct vlc_input_event_es
> *ev)
> + const struct vlc_input_event_es
> *ev,
> + const struct vlc_player_track_priv
> *trackpriv)
> {
> vlc_player_t *player = input->player;
> + if(ev->fmt->i_cat != SPU_ES &&
> + ev->fmt->i_codec != VLC_CODEC_TELETEXT)
> + return;
> switch (ev->action)
> {
> case VLC_INPUT_ES_ADDED:
> - if (input->teletext_menu)
> + {
> + if(!input->teletext_source)
> {
> - msg_Warn(player, "Can't handle more than one teletext
> menu "
> - "track. Using the last one.");
> - vlc_player_track_priv_Delete(input->teletext_menu);
> + input->teletext_source = trackpriv;
> + vlc_player_SendEvent(player, on_teletext_menu_changed,
> true);
> }
> - input->teletext_menu = vlc_player_track_priv_New(ev->id,
> ev->title,
> - ev->fmt);
> - if (!input->teletext_menu)
> - return;
> -
> - vlc_player_SendEvent(player, on_teletext_menu_changed,
> true);
> break;
> + }
> case VLC_INPUT_ES_DELETED:
> {
> - if (input->teletext_menu && input->teletext_menu->t.es_id
> == ev->id)
> + if (input->teletext_source == trackpriv)
> {
> - assert(!input->teletext_enabled);
> -
> - vlc_player_track_priv_Delete(input->teletext_menu);
> - input->teletext_menu = NULL;
> - vlc_player_SendEvent(player, on_teletext_menu_changed,
> false);
> + input->teletext_source =
> + vlc_player_FindTeletextSource(input,
> trackpriv, true);
> + if(!input->teletext_source)
> + input->teletext_source =
> + vlc_player_FindTeletextSource(input,
> trackpriv, false);
> + if(!input->teletext_source) /* no more teletext ES */
> + {
> + if(input->teletext_enabled)
> + {
> + input->teletext_enabled = false;
> + vlc_player_SendEvent(player,
> on_teletext_enabled_changed, false);
> + }
> + vlc_player_SendEvent(player,
> on_teletext_menu_changed, false);
> + }
> + else /* another teletext ES was reselected */
> + {
> + if(input->teletext_source->t.selected !=
> input->teletext_enabled)
> + vlc_player_SendEvent(player,
> on_teletext_menu_changed,
> +
> input->teletext_source->t.selected);
> + vlc_player_SendEvent(player,
> on_teletext_page_changed,
> +
> vlc_player_input_TeletextUserPage(input->teletext_source));
> + }
> }
> break;
> }
> case VLC_INPUT_ES_UPDATED:
> break;
> case VLC_INPUT_ES_SELECTED:
> + {
> + if(!input->teletext_enabled) /* we stick with the first
> selected */
> + {
> + input->teletext_source = trackpriv;
> + input->teletext_enabled = true;
> + vlc_player_SendEvent(player,
> on_teletext_enabled_changed, true);
> + vlc_player_SendEvent(player, on_teletext_page_changed,
> + vlc_player_input_TeletextUserPage(trackpriv));
> + }
> + break;
> + }
> case VLC_INPUT_ES_UNSELECTED:
> - if (input->teletext_menu->t.es_id == ev->id)
> + if(input->teletext_source == trackpriv)
> {
> - input->teletext_enabled = ev->action ==
> VLC_INPUT_ES_SELECTED;
> - vlc_player_SendEvent(player,
> on_teletext_enabled_changed,
> - input->teletext_enabled);
> + /* If there's another selected teletext, it needs to
> become source */
> + const struct vlc_player_track_priv *other =
> + vlc_player_FindTeletextSource(input,
> trackpriv, true);
> + if(other)
> + {
> + input->teletext_source = other;
> + if(!input->teletext_enabled)
> + {
> + input->teletext_enabled = true;
> + vlc_player_SendEvent(player,
> on_teletext_enabled_changed, true);
> + }
> + vlc_player_SendEvent(player,
> on_teletext_page_changed,
> + vlc_player_input_TeletextUserPage(other));
> + }
> + else
> + {
> + input->teletext_enabled = false;
> + vlc_player_SendEvent(player,
> on_teletext_enabled_changed, false);
> + }
> }
> break;
> default:
> @@ -409,14 +477,6 @@ vlc_player_input_HandleEsEvent(struct
> vlc_player_input *input,
> {
> assert(ev->id && ev->title && ev->fmt);
>
> - if (ev->fmt->i_cat == SPU_ES && ev->fmt->i_codec ==
> VLC_CODEC_TELETEXT
> - && (ev->fmt->subs.teletext.i_magazine == 1
> - || ev->fmt->subs.teletext.i_magazine > 8))
> - {
> - vlc_player_input_HandleTeletextMenu(input, ev);
> - return;
> - }
> -
> vlc_player_track_vector *vec =
> vlc_player_input_GetTrackVector(input, ev->fmt->i_cat);
> if (!vec)
> @@ -438,6 +498,7 @@ vlc_player_input_HandleEsEvent(struct
> vlc_player_input *input,
> }
> vlc_player_SendEvent(player, on_track_list_changed,
> VLC_PLAYER_LIST_ADDED, &trackpriv->t);
> + vlc_player_input_HandleTeletextMenu(input, ev, trackpriv);
> break;
> case VLC_INPUT_ES_DELETED:
> {
> @@ -445,6 +506,7 @@ vlc_player_input_HandleEsEvent(struct
> vlc_player_input *input,
> trackpriv = vlc_player_track_vector_FindById(vec, ev->id,
> &idx);
> if (trackpriv)
> {
> + vlc_player_input_HandleTeletextMenu(input, ev,
> trackpriv);
> vlc_player_SendEvent(player, on_track_list_changed,
> VLC_PLAYER_LIST_REMOVED,
> &trackpriv->t);
> vlc_vector_remove(vec, idx);
> @@ -460,6 +522,7 @@ vlc_player_input_HandleEsEvent(struct
> vlc_player_input *input,
> break;
> vlc_player_SendEvent(player, on_track_list_changed,
> VLC_PLAYER_LIST_UPDATED,
> &trackpriv->t);
> + vlc_player_input_HandleTeletextMenu(input, ev, trackpriv);
> break;
> case VLC_INPUT_ES_SELECTED:
> trackpriv = vlc_player_track_vector_FindById(vec, ev->id,
> NULL);
> @@ -469,6 +532,7 @@ vlc_player_input_HandleEsEvent(struct
> vlc_player_input *input,
> trackpriv->selected_by_user = ev->forced;
> vlc_player_SendEvent(player,
> on_track_selection_changed,
> NULL, trackpriv->t.es_id);
> + vlc_player_input_HandleTeletextMenu(input, ev,
> trackpriv);
> }
> break;
> case VLC_INPUT_ES_UNSELECTED:
> @@ -480,6 +544,7 @@ vlc_player_input_HandleEsEvent(struct
> vlc_player_input *input,
> trackpriv->selected_by_user = false;
> vlc_player_SendEvent(player,
> on_track_selection_changed,
> trackpriv->t.es_id, NULL);
> + vlc_player_input_HandleTeletextMenu(input, ev,
> trackpriv);
> }
> break;
> default:
> @@ -856,7 +921,7 @@ vlc_player_input_New(vlc_player_t *player,
> input_item_t *item)
> vlc_vector_init(&input->video_track_vector);
> vlc_vector_init(&input->audio_track_vector);
> vlc_vector_init(&input->spu_track_vector);
> - input->teletext_menu = NULL;
> + input->teletext_source = NULL;
>
> input->titles = NULL;
> input->title_selected = input->chapter_selected = 0;
> @@ -934,7 +999,7 @@ vlc_player_input_Delete(struct vlc_player_input *input)
> assert(input->video_track_vector.size == 0);
> assert(input->audio_track_vector.size == 0);
> assert(input->spu_track_vector.size == 0);
> - assert(input->teletext_menu == NULL);
> + assert(input->teletext_source == NULL);
>
> vlc_vector_destroy(&input->program_vector);
> vlc_vector_destroy(&input->video_track_vector);
> diff --git a/src/player/player.c b/src/player/player.c
> index a1a39a28f5..d70e9fe672 100644
> --- a/src/player/player.c
> +++ b/src/player/player.c
> @@ -752,25 +752,24 @@ void
> vlc_player_SetTeletextEnabled(vlc_player_t *player, bool enabled)
> {
> struct vlc_player_input *input = vlc_player_get_input_locked(player);
> - if (!input || !input->teletext_menu)
> + if (!input || !input->teletext_source)
> return;
> if (enabled)
> - vlc_player_SelectEsId(player, input->teletext_menu->t.es_id,
> + vlc_player_SelectEsId(player, input->teletext_source->t.es_id,
> VLC_PLAYER_SELECT_EXCLUSIVE);
> else
> - vlc_player_UnselectEsId(player, input->teletext_menu->t.es_id);
> + vlc_player_UnselectEsId(player, input->teletext_source->t.es_id);
> }
>
> void
> vlc_player_SelectTeletextPage(vlc_player_t *player, unsigned page)
> {
> struct vlc_player_input *input = vlc_player_get_input_locked(player);
> - if (!input || !input->teletext_menu)
> + if (!input || !input->teletext_source)
> return;
> -
> input_ControlPush(input->thread, INPUT_CONTROL_SET_VBI_PAGE,
> &(input_control_param_t) {
> - .vbi_page.id = input->teletext_menu->t.es_id,
> + .vbi_page.id = input->teletext_source->t.es_id,
> .vbi_page.page = page,
> });
> }
> @@ -779,12 +778,12 @@ void
> vlc_player_SetTeletextTransparency(vlc_player_t *player, bool enabled)
> {
> struct vlc_player_input *input = vlc_player_get_input_locked(player);
> - if (!input || !input->teletext_menu)
> + if (!input || !input->teletext_source)
> return;
>
> input_ControlPush(input->thread, INPUT_CONTROL_SET_VBI_TRANSPARENCY,
> &(input_control_param_t) {
> - .vbi_transparency.id = input->teletext_menu->t.es_id,
> + .vbi_transparency.id = input->teletext_source->t.es_id,
> .vbi_transparency.enabled = enabled,
> });
> }
> @@ -793,7 +792,7 @@ bool
> vlc_player_HasTeletextMenu(vlc_player_t *player)
> {
> struct vlc_player_input *input = vlc_player_get_input_locked(player);
> - return input && input->teletext_menu;
> + return input && input->teletext_source;
> }
>
> bool
> @@ -802,7 +801,7 @@ vlc_player_IsTeletextEnabled(vlc_player_t *player)
> struct vlc_player_input *input = vlc_player_get_input_locked(player);
> if (input && input->teletext_enabled)
> {
> - assert(input->teletext_menu);
> + assert(input->teletext_source);
> return true;
> }
> return false;
> diff --git a/src/player/player.h b/src/player/player.h
> index 4ba6e061fa..9d0182b2ac 100644
> --- a/src/player/player.h
> +++ b/src/player/player.h
> @@ -85,7 +85,7 @@ struct vlc_player_input
> vlc_player_track_vector video_track_vector;
> vlc_player_track_vector audio_track_vector;
> vlc_player_track_vector spu_track_vector;
> - struct vlc_player_track_priv *teletext_menu;
> + const struct vlc_player_track_priv *teletext_source;
Otherwise, LGTM.
You could also add a test case in the player test to avoid future breakage (since teletext is rarely tested by devs).
>
> struct vlc_player_title_list *titles;
>
> --
> 2.25.4
>
> _______________________________________________
> 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