<!DOCTYPE html>
<html>
<head>
<title></title>
</head>
<body><div><br></div>
<div>On Sun, Apr 2, 2017, at 23:09, Filip Roséen wrote:<br></div>
<blockquote type="cite"><p>Hi Thomas,<br></p><p>On 2017-03-31 15:25, Thomas Guillem wrote:<br></p><blockquote style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204, 204, 204);padding-left:1ex;color:rgb(80, 0, 80);"><pre><code style="white-space: pre;"> This event was sent only when an ES was changed by libvlc. It was not sent when
an ES was changed by the input thread. To fix this issue, listen to the
INPUT_EVENT_ES input event (that is sent when an ES is added/deleted/selected)
to detect an ES selection change.
We don't want to trigger callbacks for "video-es"/"audio-es"/"spu-es" variables
from the input_thread since it's also listening to these callbacks and we want
to avoid a selecting busy loop.
---
lib/media_player.c | 65 ++++++++++++++++++++++++---------------------
lib/media_player_internal.h | 1 +
2 files changed, 36 insertions(+), 30 deletions(-)
diff --git a/lib/media_player.c b/lib/media_player.c
index 2c5aa727fb..6ab47254f7 100644
--- a/lib/media_player.c
+++ b/lib/media_player.c
@@ -65,11 +65,6 @@ input_es_changed( vlc_object_t * p_this, char const * psz_cmd,
void *p_userdata);
static int
-input_es_selected( vlc_object_t * p_this, char const * psz_cmd,
- vlc_value_t oldval, vlc_value_t newval,
- void * p_userdata );
-
-static int
corks_changed(vlc_object_t *obj, const char *name, vlc_value_t old,
vlc_value_t cur, void *opaque);
@@ -411,6 +406,39 @@ input_event_changed( vlc_object_t * p_this, char const * psz_cmd,
event.u.media_player_chapter_changed.new_chapter = var_GetInteger( p_input, "chapter" );
libvlc_event_send( p_mi->p_event_manager, &event );
}
+ else if ( newval.i_int == INPUT_EVENT_ES )
+ {
+ /* Send ESSelected events from this callback ("intf-event") and not
+ * from "audio-es", "video-es", "spu-es" callbacks. Indeed, these
+ * callbacks are not triggered when the input_thread changes an ES
+ * while this callback is. */
+
+ int new_es[3] = { -2, -2, -2 }; /* -1 is reserved for deselect */
+ static const char *es_names[3] = { "audio-es", "video-es", "spu-es" };
+ static const int es_types[3] = { libvlc_track_audio, libvlc_track_video,
+ libvlc_track_text };</code><br></pre></blockquote><p>As these two arrays are tightly coupled, why not turn them into one array of unnamed structs having both <code style="white-space: pre;">const char*</code> and <code style="white-space: pre;">const int</code> as <i>data-members</i>?<br></p></blockquote><div><br></div>
<div>OK,<br></div>
<div><br></div>
<blockquote type="cite"><blockquote style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204, 204, 204);padding-left:1ex;color:rgb(80, 0, 80);"><pre><code style="white-space: pre;"> + /* Check if an es selection changed */
+ lock( p_mi );
+ for( int i = 0; i < 3; ++i ) /* for audio, video, spu */
+ {
+ int current_es = var_GetInteger( p_input, es_names[i] );
+ if( current_es != p_mi->selected_es[i] )
+ new_es[i] = p_mi->selected_es[i] = current_es;
+ }
+ unlock( p_mi );
+
+ /* Send the ESSelected event for each ES that were newly selected */
+ for( int i = 0; i < 3; ++i ) /* for audio, video, spu */
+ {
+ if( new_es[i] != -2 )
+ {</code><br></pre></blockquote><p>Honestly I am not a super big fan of using <code style="white-space: pre;">-2</code> as a magic number, but my soul would rest a little easier if it was at least denoted by a named constant (to hide the mischief).<br></p></blockquote><div><br></div>
<div>Then I have to declare a define / const for the whole file, I don't know.<br></div>
<div><br></div>
<blockquote type="cite"><blockquote style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204, 204, 204);padding-left:1ex;color:rgb(80, 0, 80);"><pre><code style="white-space: pre;"> + event.type = libvlc_MediaPlayerESSelected;
+ event.u.media_player_es_changed.i_type = es_types[i];
+ event.u.media_player_es_changed.i_id = new_es[i];
+ libvlc_event_send( p_mi->p_event_manager, &event );
+ }
+ }
+ }
return VLC_SUCCESS;
}
@@ -475,25 +503,6 @@ static int input_es_changed( vlc_object_t *p_this,
return VLC_SUCCESS;
}
-static int
-input_es_selected( vlc_object_t * p_this, char const * psz_cmd,
- vlc_value_t oldval, vlc_value_t newval,
- void * p_userdata )
-{
- VLC_UNUSED(p_this);
- VLC_UNUSED(oldval);
- libvlc_media_player_t *mp = p_userdata;
- libvlc_event_t event;
-
- event.type = libvlc_MediaPlayerESSelected;
- event.u.media_player_es_changed.i_type = track_type_from_name(psz_cmd);
- event.u.media_player_es_changed.i_id = newval.i_int;
-
- libvlc_event_send(mp->p_event_manager, &event);
-
- return VLC_SUCCESS;
-}
-
/**************************************************************************
* Snapshot Taken Event.
*
@@ -916,9 +925,6 @@ static void add_es_callbacks( input_thread_t *p_input_thread, libvlc_media_playe
var_AddListCallback( p_input_thread, "video-es", input_es_changed, p_mi );
var_AddListCallback( p_input_thread, "audio-es", input_es_changed, p_mi );
var_AddListCallback( p_input_thread, "spu-es", input_es_changed, p_mi );
- var_AddCallback( p_input_thread, "video-es", input_es_selected, p_mi );
- var_AddCallback( p_input_thread, "audio-es", input_es_selected, p_mi );
- var_AddCallback( p_input_thread, "spu-es", input_es_selected, p_mi );
}
static void del_es_callbacks( input_thread_t *p_input_thread, libvlc_media_player_t *p_mi )
@@ -926,9 +932,6 @@ static void del_es_callbacks( input_thread_t *p_input_thread, libvlc_media_playe
var_DelListCallback( p_input_thread, "video-es", input_es_changed, p_mi );
var_DelListCallback( p_input_thread, "audio-es", input_es_changed, p_mi );
var_DelListCallback( p_input_thread, "spu-es", input_es_changed, p_mi );
- var_DelCallback( p_input_thread, "video-es", input_es_selected, p_mi );
- var_DelCallback( p_input_thread, "audio-es", input_es_selected, p_mi );
- var_DelCallback( p_input_thread, "spu-es", input_es_selected, p_mi );
}
/**************************************************************************
@@ -958,6 +961,8 @@ int libvlc_media_player_play( libvlc_media_player_t *p_mi )
return -1;
}
+ memset( p_mi->selected_es, -2, sizeof(p_mi->selected_es) );
+</code><br></pre></blockquote><p>The above is not correct as it will set every <i>byte</i> in <code style="white-space: pre;">p_mi->selected_is</code> to <code style="white-space: pre;">-2</code>, it will not set each of the individual elements in the array to <code style="white-space: pre;">-2</code> (as <code style="white-space: pre;">sizeof p_mi->selected_es[0] > 1</code>).<br></p></blockquote><div><br></div>
<div>Oops, yes, will fix.<br></div>
<div><br></div>
<blockquote type="cite"><blockquote style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204, 204, 204);padding-left:1ex;color:rgb(80, 0, 80);"><pre><code style="white-space: pre;"> media_attach_preparsed_event( p_mi->p_md );
p_input_thread = input_Create( p_mi, p_mi->p_md->p_input_item, NULL,
diff --git a/lib/media_player_internal.h b/lib/media_player_internal.h
index 1eca3c3f76..a5fa20dc0a 100644
--- a/lib/media_player_internal.h
+++ b/lib/media_player_internal.h
@@ -55,6 +55,7 @@ struct libvlc_media_player_t
libvlc_event_manager_t * p_event_manager;
libvlc_state_t state;
vlc_viewpoint_t viewpoint;
+ int selected_es[3];
};
/* Media player - audio, video */
--
2.11.0</code><br></pre></blockquote><p>I do not understand why <i>libvlc</i> must “cache” the selected ES to prevent a selection event of the same ES bubbling up twice. If we have code that spawns an “ES selected” event for an ES that is already selected (without explicit interaction), that code should be fixed.<br></p></blockquote><div><div><div><br></div>
</div>
<div>This commit is more a hack for 3.0. We may want to rewrite event handling for 4.0 or 5.0. We need to "cache" because this event is not sent only for selected es events.<br></div>
<div><br></div>
</div>
<blockquote type="cite"><p>If all event-posting is correct in the core, why is it not enough to simply “forward” these events as they come in (instead of having a cache filtering out duplicates)?<br></p></blockquote><div><br></div>
<div>Unfortunately, no, see patch comments and commit log.<br></div>
<div><br></div>
<blockquote type="cite"><p><div>Best Regards,<br></div>
<div> Filip<br></div>
</p><div><u>_______________________________________________</u><br></div>
<div>vlc-devel mailing list<br></div>
<div>To unsubscribe or modify your subscription options:<br></div>
<div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div>
</blockquote><div><br></div>
</body>
</html>