[vlc-devel] [3.0 PATCH] input: display OSD from NAV controls

[Thomas Guillem]

On Thu, Apr 5, 2018, at 11:44, Rémi Denis-Courmont wrote:
> Le 5 avril 2018 12:08:40 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
> >
> >On Thu, Apr 5, 2018, at 10:32, Rémi Denis-Courmont wrote:
> >> Le 5 avril 2018 09:19:05 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a
> >écrit :
> >> >Sorry in advance for this ugly hack. I didn't had the time to merge
> >the
> >> >code of
> >> >the hotkeys module as planned for 3.0. This will be done in 4.0 into
> >> >the new
> >> >input manager.
> >> >
> >> >In the meantime, it's quite important to have UI feedback when
> >> >seeking/jumping
> >> >from the arrow keys. Indeed, users won't understand why the OSD is
> >> >showing when
> >> >pressing Ctrl + arrow keys and not when pressing arrow keys alone.
> >> >
> >> >This hacks use the vlc_object_find_name() (that should be removed
> >for
> >> >4.0) to
> >> >detect if the "hotkeys" module is running and forward the commands
> >to
> >> >this
> >> >module that will show the OSD.
> >> >
> >> >Why not calling OSD functions directly from input.c ? Because we
> >need
> >> >to use
> >> >the same SPU chan for sliders (otherwise, you can end up showing 2
> >> >overlapping
> >> >sliders).
> >> >---
> >> > src/input/input.c  | 18 ++++++++++++++++++
> >> > src/misc/objects.c |  4 ++--
> >> > 2 files changed, 20 insertions(+), 2 deletions(-)
> >> >
> >> >diff --git a/src/input/input.c b/src/input/input.c
> >> >index f4d49b809f..ddf62746ff 100644
> >> >--- a/src/input/input.c
> >> >+++ b/src/input/input.c
> >> >@@ -45,6 +45,7 @@
> >> > #include "stream.h"
> >> > 
> >> > #include <vlc_aout.h>
> >> >+#include <vlc_actions.h>
> >> > #include <vlc_sout.h>
> >> > #include <vlc_dialog.h>
> >> > #include <vlc_url.h>
> >> >@@ -1774,6 +1775,23 @@ static void ControlNav( input_thread_t
> >*p_input,
> >> >int i_type )
> >> >         return;
> >> >     }
> >> > 
> >> >+    /* HACK: use hotkeys module directly  since it can handle OSD.
> >> >+     * TODO: merge hotkeys module in future input manager. */
> >> >+    vlc_object_t *p_obj = vlc_object_find_name(
> >p_input->obj.libvlc,
> >> >"hotkeys" );
> >> >+    if( p_obj )
> >> >+    {
> >> >+        if( seek_direction != 0 )
> >> >+            var_SetInteger( p_input->obj.libvlc, "key-action",
> >> >+                            seek_direction == 1 ?
> >> >ACTIONID_JUMP_FORWARD_SHORT
> >> >+                                                :
> >> >ACTIONID_JUMP_BACKWARD_SHORT );
> >> >+        else
> >> >+            var_SetInteger( p_input->obj.libvlc, "key-action",
> >> >+                            vol_direction == 1 ? ACTIONID_VOL_UP
> >> >+                                               : ACTIONID_VOL_DOWN
> >);
> >> >+        vlc_object_release( p_obj );
> >> >+        return;
> >> >+    } /* else, no hotkeys module: send command directly */
> >> >+
> >> >/* Seek or change volume if the input doesn't have navigation or
> >> >viewpoint */
> >> >     if( seek_direction != 0 )
> >> >     {
> >> >diff --git a/src/misc/objects.c b/src/misc/objects.c
> >> >index d1ea869410..aee3356454 100644
> >> >--- a/src/misc/objects.c
> >> >+++ b/src/misc/objects.c
> >> >@@ -388,8 +388,8 @@ vlc_object_t *vlc_object_find_name( vlc_object_t
> >> >*p_this, const char *psz_name )
> >> >/* This was officially deprecated on August 19 2009. For the
> >> >convenience of
> >> >    * wannabe code janitors, this is the list of names that remain
> >used
> >> >      * and unfixed since then. */
> >> >-    static const char bad[][5] = { "v4l2", "zvbi" };
> >> >-    if( bsearch( psz_name, bad, 2, 5, (void *)strcmp ) == NULL )
> >> >+    static const char bad[][8] = { "hotkeys", "v4l2", "zvbi" };
> >> >+    if( bsearch( psz_name, bad, 3, 8, (void *)strcmp ) == NULL )
> >> >         return NULL;
> >> > msg_Err( p_this, "looking for object \"%s\"... FIXME XXX", psz_name
> >);
> >> > #endif
> >> >-- 
> >> >2.11.0
> >> >
> >> >_______________________________________________
> >> >vlc-devel mailing list
> >> >To unsubscribe or modify your subscription options:
> >> >https://mailman.videolan.org/listinfo/vlc-devel
> >> 
> >> This would route the navigation controls to the wrong input if there
> >are 
> >> more than one. Which seems far worse than a cosmetic lack of OSD.
> >
> >Good point, I'll add the following check:
> >var_GetAddress( p_input->obj.parent, "input-current" ) == p_input;
> >
> >Either, the parent of this input is the playlist or something else (the
> >libvlc mediaplayer). In both case, fetching "input-current" will
> >indicate if this input is handled by the hotkeys module.
> >
> >> 
> >> There are reasons why find-name has been deprecated for over 8 years.
> >
> >I tried to remove this function completely for VLC 3.0. It's only used
> >by v4l2 (and zvbi on macOS) now. I already know this is a forbidden
> >function and that this patch is ugly. But, sometimes, having a coherent
> >app is more important than this kind of concern, no ? I precise that I
> >only want this patch for 3.0.
> >
> >> -- 
> >> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
> >excuser 
> >> ma brièveté.
> >> _______________________________________________
> >> vlc-devel mailing list
> >> To unsubscribe or modify your subscription options:
> >> https://mailman.videolan.org/listinfo/vlc-devel
> >_______________________________________________
> >vlc-devel mailing list
> >To unsubscribe or modify your subscription options:
> >https://mailman.videolan.org/listinfo/vlc-devel
> 
> That is yet worse. It becomes racy (since the variable is only meant for 
> callbacks) and remains inconsistent (since it won't always solve the 
> targetted issue).
> 
> In a *stable* branch, *stability* is far more important than 
> consistency. The time to worry about consistency was before the code 
> freeze.

But new issues that we need to fix are discovered after the freeze because many users start to test our code. It's a chicken egg situation...

> 
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser 
> ma brièveté.