[vlc-devel] [PATCH v3 2/8] libvlc: player: fix racy tests

[Rémi Denis-Courmont]

Nit: I don't think it was " racy". A live loop is not a race condition - the result does not vary based on a race.

Le 20 mai 2019 18:35:11 GMT+03:00, Romain Vimont <rom1v at videolabs.io> a écrit :
>In order to wait for a specific state, libvlc player tests used live
>loops to read the player state.
>
>Listen to state changes instead.
>---
> test/Makefile.am           |  2 +-
> test/libvlc/media_player.c | 85 ++++++++++++++++++++++++--------------
> 2 files changed, 55 insertions(+), 32 deletions(-)
>
>diff --git a/test/Makefile.am b/test/Makefile.am
>index 8343bea501..625f8d3453 100644
>--- a/test/Makefile.am
>+++ b/test/Makefile.am
>@@ -98,7 +98,7 @@ test_libvlc_media_list_player_LDADD = $(LIBVLC)
> test_libvlc_media_list_SOURCES = libvlc/media_list.c
> test_libvlc_media_list_LDADD = $(LIBVLC)
> test_libvlc_media_player_SOURCES = libvlc/media_player.c
>-test_libvlc_media_player_LDADD = $(LIBVLC)
>+test_libvlc_media_player_LDADD = $(LIBVLCCORE) $(LIBVLC)
> test_libvlc_media_discoverer_SOURCES = libvlc/media_discoverer.c
> test_libvlc_media_discoverer_LDADD = $(LIBVLC)
> test_libvlc_renderer_discoverer_SOURCES = libvlc/renderer_discoverer.c
>diff --git a/test/libvlc/media_player.c b/test/libvlc/media_player.c
>index 5417fd7561..2489e6f5fa 100644
>--- a/test/libvlc/media_player.c
>+++ b/test/libvlc/media_player.c
>@@ -21,30 +21,61 @@
>**********************************************************************/
> 
> #include "test.h"
>+#include <vlc_common.h>
> 
>-static void wait_playing(libvlc_media_player_t *mp)
>+static void on_event(const struct libvlc_event_t *event, void *data)
> {
>-    libvlc_state_t state;
>-    do {
>-        state = libvlc_media_player_get_state (mp);
>-    } while(state != libvlc_Playing &&
>-            state != libvlc_Error &&
>-            state != libvlc_Ended );
>-
>-    state = libvlc_media_player_get_state (mp);
>-    assert(state == libvlc_Playing || state == libvlc_Ended);
>+    (void) event;
>+    vlc_sem_t *sem = data;
>+    vlc_sem_post(sem);
> }
> 
>-static void wait_paused(libvlc_media_player_t *mp)
>+static void play_and_wait(libvlc_media_player_t *mp)
> {
>-    libvlc_state_t state;
>-    do {
>-        state = libvlc_media_player_get_state (mp);
>-    } while(state != libvlc_Paused &&
>-            state != libvlc_Ended );
>-
>-    state = libvlc_media_player_get_state (mp);
>-    assert(state == libvlc_Paused || state == libvlc_Ended);
>+    libvlc_event_manager_t *em =
>libvlc_media_player_event_manager(mp);
>+
>+    vlc_sem_t sem;
>+    vlc_sem_init(&sem, 0);
>+
>+    int res;
>+    res = libvlc_event_attach(em, libvlc_MediaPlayerPlaying, on_event,
>&sem);
>+    assert(!res);
>+
>+    libvlc_media_player_play(mp);
>+
>+    test_log("Waiting for playing\n");
>+    vlc_sem_wait(&sem);
>+
>+    libvlc_event_detach(em, libvlc_MediaPlayerPlaying, on_event,
>&sem);
>+
>+    vlc_sem_destroy(&sem);
>+}
>+
>+static void pause_and_wait(libvlc_media_player_t *mp)
>+{
>+    libvlc_event_manager_t *em =
>libvlc_media_player_event_manager(mp);
>+
>+    vlc_sem_t sem;
>+    vlc_sem_init(&sem, 0);
>+
>+    int res;
>+    res = libvlc_event_attach(em, libvlc_MediaPlayerPaused, on_event,
>&sem);
>+    assert(!res);
>+    res = libvlc_event_attach(em, libvlc_MediaPlayerEndReached,
>on_event, &sem);
>+    assert(!res);
>+
>+    libvlc_media_player_set_pause(mp, true);
>+
>+    /* the end may have been already reached before attaching the
>event */
>+    if (libvlc_media_player_get_state(mp) == libvlc_Playing)
>+    {
>+        test_log("Waiting for pause\n");
>+        vlc_sem_wait(&sem);
>+    }
>+
>+    vlc_sem_destroy(&sem);
>+    libvlc_event_detach(em, libvlc_MediaPlayerPaused, on_event, &sem);
>+    libvlc_event_detach(em, libvlc_MediaPlayerEndReached, on_event,
>&sem);
> }
> 
>/* Test a bunch of A/V properties. This most does nothing since the
>current
>@@ -111,9 +142,7 @@ static void test_media_player_set_media(const
>char** argv, int argc)
> 
>     libvlc_media_release (md);
> 
>-    libvlc_media_player_play (mp);
>-
>-    wait_playing (mp);
>+    play_and_wait(mp);
> 
>     libvlc_media_player_stop (mp);
>     libvlc_media_player_release (mp);
>@@ -140,9 +169,7 @@ static void test_media_player_play_stop(const
>char** argv, int argc)
> 
>     libvlc_media_release (md);
> 
>-    libvlc_media_player_play (mi);
>-
>-    wait_playing (mi);
>+    play_and_wait(mi);
> 
>     libvlc_media_player_stop (mi);
>     libvlc_media_player_release (mi);
>@@ -172,14 +199,10 @@ static void test_media_player_pause_stop(const
>char** argv, int argc)
>     test_audio_video(mi);
>     test_role(mi);
> 
>-    libvlc_media_player_play (mi);
>-    test_log ("Waiting for playing\n");
>-    wait_playing (mi);
>+    play_and_wait(mi);
>     test_audio_video(mi);
> 
>-    libvlc_media_player_set_pause (mi, true);
>-    test_log ("Waiting for pause\n");
>-    wait_paused (mi);
>+    pause_and_wait(mi);
>     test_audio_video(mi);
> 
>     libvlc_media_player_stop (mi);
>-- 
>2.20.1
>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190520/fa57648c/attachment.html>