[vlc-devel] [PATCH 2/2] lib: media_player: fix libvlc_MediaPlayerMediaChanged event

[Romain Vimont]

This assumes that the player only plays items created from a
libvlc_media_t.

If it is guaranteed to always be the case, then LGTM.

On Fri, Sep 18, 2020 at 02:29:32PM +0200, Thomas Guillem wrote:
> - Don't override the media set by the user from
>   on_current_media_changed().
> 
> - Don't create a new media from an input_item. This will prevent to send
>   media events with a media pointer that is unknown to the caller.
> 
> This callback is now always sending a media that is set with
> libvlc_media_player_set_media().
> 
> Since the player is now fully asynchronous, a call to set_media() +
> play() will be processed asynchronously if an older media is playing.
> The user might want to know when its new media is actually playing.
> Listening to the libvlc_MediaPlayerMediaChanged event is only way to do so.
> 
> Here is an example:
> 
> 1. set_media(A) && play()
>   -> get_media() -> A
> 2. set_media(B) && play()
>   -> get_media() -> B
> 3. libvlc_MediaPlayerMediaChanged(A)
>   -> get_media() -> B
> 4. libvlc_MediaPlayerMediaChanged(B)
>   -> get_media() -> B
> ---
>  include/vlc/libvlc_media_player.h | 11 ++++++++++-
>  lib/media.c                       |  1 +
>  lib/media_player.c                | 28 ++++++----------------------
>  3 files changed, 17 insertions(+), 23 deletions(-)
> 
> diff --git a/include/vlc/libvlc_media_player.h b/include/vlc/libvlc_media_player.h
> index 7dec44f1af2..8f6447574c9 100644
> --- a/include/vlc/libvlc_media_player.h
> +++ b/include/vlc/libvlc_media_player.h
> @@ -197,16 +197,25 @@ LIBVLC_API void libvlc_media_player_retain( libvlc_media_player_t *p_mi );
>   * Set the media that will be used by the media_player. If any,
>   * previous md will be released.
>   *
> + * \note The user should listen to the libvlc_MediaPlayerMediaChanged event, to
> + * know when the new media is actually used by the player (or to known that the
> + * older media is no longuer used).
> + *
>   * \param p_mi the Media Player
>   * \param p_md the Media. Afterwards the p_md can be safely
>   *        destroyed.
>   */
>  LIBVLC_API void libvlc_media_player_set_media( libvlc_media_player_t *p_mi,
> -                                                   libvlc_media_t *p_md );
> +                                               libvlc_media_t *p_md );
>  
>  /**
>   * Get the media used by the media_player.
>   *
> + * \warning Calling this function just after libvlc_media_player_set_media()
> + * will return the media that was just set, but this media might not be
> + * currently used internally by the player. To detect such case, the user
> + * should listen to the libvlc_MediaPlayerMediaChanged event.
> + *
>   * \param p_mi the Media Player
>   * \return the media associated with p_mi, or NULL if no
>   *         media is associated
> diff --git a/lib/media.c b/lib/media.c
> index e4c0909c165..7d826783334 100644
> --- a/lib/media.c
> +++ b/lib/media.c
> @@ -483,6 +483,7 @@ libvlc_media_t * libvlc_media_new_from_input_item(
>      libvlc_event_manager_init( &p_md->event_manager, p_md );
>  
>      input_item_Hold( p_md->p_input_item );
> +    p_md->p_input_item->owner = p_md;
>  
>      install_input_item_observer( p_md );
>  
> diff --git a/lib/media_player.c b/lib/media_player.c
> index 4db7f96af05..de7df0b98cb 100644
> --- a/lib/media_player.c
> +++ b/lib/media_player.c
> @@ -64,34 +64,18 @@ on_current_media_changed(vlc_player_t *player, input_item_t *new_media,
>  
>      libvlc_media_player_t *mp = data;
>  
> -    libvlc_media_t *md = mp->p_md;
> -
> -    input_item_t *media = md ? md->p_input_item : NULL;
> -    if (new_media == media)
> -        /* no changes */
> -        return;
> -
> -    if (md)
> -        media_detach_preparsed_event(md);
> -
> -    if (new_media)
> +    libvlc_media_t *libmedia;
> +    if (new_media != NULL)
>      {
> -        mp->p_md = libvlc_media_new_from_input_item(mp->p_libvlc_instance,
> -                                                    new_media);
> -        if (!mp->p_md)
> -            /* error already printed by the function call */
> -            return;
> -
> -        media_attach_preparsed_event(mp->p_md);
> +        libmedia = new_media->owner;
> +        assert(libmedia != NULL);
>      }
>      else
> -        mp->p_md = NULL;
> -
> -    libvlc_media_release(md);
> +        libmedia = NULL;
>  
>      libvlc_event_t event;
>      event.type = libvlc_MediaPlayerMediaChanged;
> -    event.u.media_player_media_changed.new_media = mp->p_md;
> +    event.u.media_player_media_changed.new_media = libmedia;
>      libvlc_event_send(&mp->event_manager, &event);
>  }
>  
> -- 
> 2.28.0
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel