All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Input: Add missing event codes for common IR remote buttons
@ 2018-11-03 14:55 Derek Kelly
  2018-11-05 20:53 ` Sean Young
  2018-11-13 13:04 ` Bastien Nocera
  0 siblings, 2 replies; 10+ messages in thread
From: Derek Kelly @ 2018-11-03 14:55 UTC (permalink / raw)
  To: linux-input; +Cc: sean, mchehab+samsung, linux-media

The following patch adds event codes for common buttons found on various
provider and universal remote controls. They represent functions not
covered by existing event codes. Once added, rc_keymaps can be updated
accordingly where applicable.

v2 changes:
Renamed KEY_SYSTEM to KEY_SYSTEM_MENU to avoid conflict with powerpc
KEY_SYSTEM define.

Signed-off-by: Derek Kelly <user.vdr@gmail.com>
---
 include/uapi/linux/input-event-codes.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 53fbae27b280..a15fd3c944d2 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -689,6 +689,19 @@
 #define BTN_TRIGGER_HAPPY39		0x2e6
 #define BTN_TRIGGER_HAPPY40		0x2e7
 
+/* Remote control buttons found across provider & universal remotes */
+#define KEY_LIVE_TV			0x2e8	/* Jump to live tv viewing */
+#define KEY_OPTIONS			0x2e9	/* Jump to options */
+#define KEY_INTERACTIVE			0x2ea	/* Jump to interactive system/menu/item */
+#define KEY_MIC_INPUT			0x2eb	/* Trigger MIC input/listen mode */
+#define KEY_SCREEN_INPUT		0x2ec	/* Open on-screen input system */
+#define KEY_SYSTEM_MENU			0x2ed	/* Open systems menu/display */
+#define KEY_SERVICES			0x2ee	/* Access services */
+#define KEY_DISPLAY_FORMAT		0x2ef	/* Cycle display formats */
+#define KEY_PIP				0x2f0	/* Toggle Picture-in-Picture on/off */
+#define KEY_PIP_SWAP			0x2f1	/* Swap contents between main view and PIP window */
+#define KEY_PIP_POSITION		0x2f2	/* Cycle PIP window position */
+
 /* We avoid low common keys in module aliases so they don't get huge. */
 #define KEY_MIN_INTERESTING	KEY_MUTE
 #define KEY_MAX			0x2ff
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Input: Add missing event codes for common IR remote buttons
  2018-11-03 14:55 [PATCH v2] Input: Add missing event codes for common IR remote buttons Derek Kelly
@ 2018-11-05 20:53 ` Sean Young
  2018-11-13 13:04 ` Bastien Nocera
  1 sibling, 0 replies; 10+ messages in thread
From: Sean Young @ 2018-11-05 20:53 UTC (permalink / raw)
  To: Derek Kelly; +Cc: linux-input, mchehab+samsung, linux-media

On Sat, Nov 03, 2018 at 07:55:32AM -0700, Derek Kelly wrote:
> The following patch adds event codes for common buttons found on various
> provider and universal remote controls. They represent functions not
> covered by existing event codes. Once added, rc_keymaps can be updated
> accordingly where applicable.
> 
> v2 changes:
> Renamed KEY_SYSTEM to KEY_SYSTEM_MENU to avoid conflict with powerpc
> KEY_SYSTEM define.
> 
> Signed-off-by: Derek Kelly <user.vdr@gmail.com>

Reviewed-by: Sean Young <sean@mess.org>

There are many remotes with these buttons, this is a very useful addition.

Thanks,

Sean

> ---
>  include/uapi/linux/input-event-codes.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 53fbae27b280..a15fd3c944d2 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -689,6 +689,19 @@
>  #define BTN_TRIGGER_HAPPY39		0x2e6
>  #define BTN_TRIGGER_HAPPY40		0x2e7
>  
> +/* Remote control buttons found across provider & universal remotes */
> +#define KEY_LIVE_TV			0x2e8	/* Jump to live tv viewing */
> +#define KEY_OPTIONS			0x2e9	/* Jump to options */
> +#define KEY_INTERACTIVE			0x2ea	/* Jump to interactive system/menu/item */
> +#define KEY_MIC_INPUT			0x2eb	/* Trigger MIC input/listen mode */
> +#define KEY_SCREEN_INPUT		0x2ec	/* Open on-screen input system */
> +#define KEY_SYSTEM_MENU			0x2ed	/* Open systems menu/display */
> +#define KEY_SERVICES			0x2ee	/* Access services */
> +#define KEY_DISPLAY_FORMAT		0x2ef	/* Cycle display formats */
> +#define KEY_PIP				0x2f0	/* Toggle Picture-in-Picture on/off */
> +#define KEY_PIP_SWAP			0x2f1	/* Swap contents between main view and PIP window */
> +#define KEY_PIP_POSITION		0x2f2	/* Cycle PIP window position */
> +
>  /* We avoid low common keys in module aliases so they don't get huge. */
>  #define KEY_MIN_INTERESTING	KEY_MUTE
>  #define KEY_MAX			0x2ff
> -- 
> 2.19.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Input: Add missing event codes for common IR remote buttons
  2018-11-03 14:55 [PATCH v2] Input: Add missing event codes for common IR remote buttons Derek Kelly
  2018-11-05 20:53 ` Sean Young
@ 2018-11-13 13:04 ` Bastien Nocera
  2018-11-13 16:20   ` VDR User
  1 sibling, 1 reply; 10+ messages in thread
From: Bastien Nocera @ 2018-11-13 13:04 UTC (permalink / raw)
  To: Derek Kelly, linux-input; +Cc: sean, mchehab+samsung, linux-media

On Sat, 2018-11-03 at 07:55 -0700, Derek Kelly wrote:
> The following patch adds event codes for common buttons found on
> various
> provider and universal remote controls. They represent functions not
> covered by existing event codes. Once added, rc_keymaps can be
> updated
> accordingly where applicable.

Would be great to have more than "those are used", such as knowing how
they are labeled, both with text and/or icons, and an explanation as to
why a particular existing key isn't usable.

> v2 changes:
> Renamed KEY_SYSTEM to KEY_SYSTEM_MENU to avoid conflict with powerpc
> KEY_SYSTEM define.
> 
> Signed-off-by: Derek Kelly <user.vdr@gmail.com>
> ---
>  include/uapi/linux/input-event-codes.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/include/uapi/linux/input-event-codes.h
> b/include/uapi/linux/input-event-codes.h
> index 53fbae27b280..a15fd3c944d2 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -689,6 +689,19 @@
>  #define BTN_TRIGGER_HAPPY39		0x2e6
>  #define BTN_TRIGGER_HAPPY40		0x2e7
>  
> +/* Remote control buttons found across provider & universal remotes */
> +#define KEY_LIVE_TV			0x2e8	/* Jump to live tv viewing */

KEY_TV?

> +#define KEY_OPTIONS			0x2e9	/* Jump to options */

KEY_OPTION?

> +#define KEY_INTERACTIVE			0x2ea	/* Jump to interactive system/menu/item */
> +#define KEY_MIC_INPUT			0x2eb	/* Trigger MIC input/listen mode */

KEY_MICMUTE?

> +#define KEY_SCREEN_INPUT		0x2ec	/* Open on-screen input system */

KEY_SWITCHVIDEOMODE?

> +#define KEY_SYSTEM_MENU			0x2ed	/* Open systems menu/display */

KEY_MENU?

> +#define KEY_SERVICES			0x2ee	/* Access services */
> +#define KEY_DISPLAY_FORMAT		0x2ef	/* Cycle display formats */

KEY_CONTEXT_MENU?

> +#define KEY_PIP				0x2f0	/* Toggle Picture-in-Picture on/off */
> +#define KEY_PIP_SWAP			0x2f1	/* Swap contents between main view and PIP window */
> +#define KEY_PIP_POSITION		0x2f2	/* Cycle PIP window position */
> +
>  /* We avoid low common keys in module aliases so they don't get huge. */
>  #define KEY_MIN_INTERESTING	KEY_MUTE
>  #define KEY_MAX			0x2ff

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Input: Add missing event codes for common IR remote buttons
  2018-11-13 13:04 ` Bastien Nocera
@ 2018-11-13 16:20   ` VDR User
  2019-01-19  8:52     ` Dmitry Torokhov
  0 siblings, 1 reply; 10+ messages in thread
From: VDR User @ 2018-11-13 16:20 UTC (permalink / raw)
  To: hadess
  Cc: linux-input, Sean Young, mchehab+samsung, mailing list: linux-media

> On Sat, 2018-11-03 at 07:55 -0700, Derek Kelly wrote:
> > The following patch adds event codes for common buttons found on
> > various
> > provider and universal remote controls. They represent functions not
> > covered by existing event codes. Once added, rc_keymaps can be
> > updated
> > accordingly where applicable.
>
> Would be great to have more than "those are used", such as knowing how
> they are labeled, both with text and/or icons, and an explanation as to
> why a particular existing key isn't usable.

Hi Bastien,

Text & icons may vary from remote to remote but the purpose/function
of those buttons is basically the same. As explained, the defines in
this patch represent functions not already addressed by other defines.
See below for more detail. The one thing I will add that I probably
should've mentioned is that these defines focus on media/htpc/stb. If
you're not aware, Linux has become a common choice for these types of
systems thanks to the popularity of software like Plex, Kodi, Mythtv,
VDR, etc. Lastly, all these represent *common* functions in this area.
Please keep this in mind as you read further.

> > +/* Remote control buttons found across provider & universal remotes */
> > +#define KEY_LIVE_TV                  0x2e8   /* Jump to live tv viewing */
>
> KEY_TV?

KEY_TV selects TV as a *input source* the same as KEY_VCR, KEY_SAT,
KEY_CD, etc. whereas KEY_LIVE_TV jumps directly to live tv as opposed
to local/networked media playback, dvr playback, etc.

> > +#define KEY_OPTIONS                  0x2e9   /* Jump to options */
>
> KEY_OPTION?

Software vs. media playback options.

> > +#define KEY_INTERACTIVE                      0x2ea   /* Jump to interactive system/menu/item */
> > +#define KEY_MIC_INPUT                        0x2eb   /* Trigger MIC input/listen mode */
>
> KEY_MICMUTE?

This button doesn't mute the mic, in fact it does the opposite. The
mic is off until you press this button, thus triggering MIC
input/listen mode and allowing the user to speak his commands. It
automatically shuts off after X seconds of silence.

> > +#define KEY_SCREEN_INPUT             0x2ec   /* Open on-screen input system */
>
> KEY_SWITCHVIDEOMODE?

KEY_SWITCHVIDEOMODE is used for "Cycle between available video outputs
(Monitor/LCD/TV-out/etc) ". This is poorly labeled in my opinion and
should've been called KEY_SWITCHVIDEOOUTPUT or something similar.
"Video mode" typically refers to something entirely different - how
video is presented on the display, not what physical display you're
using. KEY_SCREEN_INPUT is used to bring up things like an on-screen
keyboard or other on-onscreen user input method.

> > +#define KEY_SYSTEM_MENU                      0x2ed   /* Open systems menu/display */
>
> KEY_MENU?

Systems menus as pertains to DVB. KEY_MENU is generic and having only
one `menu` option is problematic when you have different types of
menus which aren't accessible from each other.

> > +#define KEY_SERVICES                 0x2ee   /* Access services */
> > +#define KEY_DISPLAY_FORMAT           0x2ef   /* Cycle display formats */
>
> KEY_CONTEXT_MENU?

KEY_DISPLAY_FORMAT doesn't open any menus and is used to cycle through
how video is displayed on-screen to the user; full, zoomed,
letterboxed, stretched, etc. KEY_CONTEXT_MENU would be for something
like bringing up a playback menu where you'd set things like
upscaling, deinterlacing, audio mixdown/mixup, etc.

> > +#define KEY_PIP                              0x2f0   /* Toggle Picture-in-Picture on/off */
> > +#define KEY_PIP_SWAP                 0x2f1   /* Swap contents between main view and PIP window */
> > +#define KEY_PIP_POSITION             0x2f2   /* Cycle PIP window position */
> > +
> >  /* We avoid low common keys in module aliases so they don't get huge. */
> >  #define KEY_MIN_INTERESTING  KEY_MUTE
> >  #define KEY_MAX                      0x2ff
>

Hopefully that makes things more clear. This patch helps users map
common (media/htpc/stb) remote control buttons directly to their real
functions as opposed to mapping them to some random unrelated & unused
event, which can be both confusing and problematic on systems where
both remote controls and say bluetooth keyboards are used.

Best regards,
Derek

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Input: Add missing event codes for common IR remote buttons
  2018-11-13 16:20   ` VDR User
@ 2019-01-19  8:52     ` Dmitry Torokhov
  2019-01-23  7:50       ` VDR User
  0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Torokhov @ 2019-01-19  8:52 UTC (permalink / raw)
  To: VDR User
  Cc: hadess, linux-input, Sean Young, mchehab+samsung,
	mailing list: linux-media

Hi Derek,

On Tue, Nov 13, 2018 at 08:20:22AM -0800, VDR User wrote:
> > On Sat, 2018-11-03 at 07:55 -0700, Derek Kelly wrote:
> > > The following patch adds event codes for common buttons found on
> > > various
> > > provider and universal remote controls. They represent functions not
> > > covered by existing event codes. Once added, rc_keymaps can be
> > > updated
> > > accordingly where applicable.
> >
> > Would be great to have more than "those are used", such as knowing how
> > they are labeled, both with text and/or icons, and an explanation as to
> > why a particular existing key isn't usable.
> 
> Hi Bastien,
> 
> Text & icons may vary from remote to remote but the purpose/function
> of those buttons is basically the same. As explained, the defines in
> this patch represent functions not already addressed by other defines.
> See below for more detail. The one thing I will add that I probably
> should've mentioned is that these defines focus on media/htpc/stb. If
> you're not aware, Linux has become a common choice for these types of
> systems thanks to the popularity of software like Plex, Kodi, Mythtv,
> VDR, etc. Lastly, all these represent *common* functions in this area.
> Please keep this in mind as you read further.
> 
> > > +/* Remote control buttons found across provider & universal remotes */
> > > +#define KEY_LIVE_TV                  0x2e8   /* Jump to live tv viewing */
> >
> > KEY_TV?
> 
> KEY_TV selects TV as a *input source* the same as KEY_VCR, KEY_SAT,
> KEY_CD, etc. whereas KEY_LIVE_TV jumps directly to live tv as opposed
> to local/networked media playback, dvr playback, etc.

I do not quite grasp the distinction. KEY_TV to select and play
broadcast TV, KEY_TV2 to switch TV input to cable.

> 
> > > +#define KEY_OPTIONS                  0x2e9   /* Jump to options */
> >
> > KEY_OPTION?
> 
> Software vs. media playback options.

This seems application control key. Do you really need both KEY_OPTION
and KEY_OPTIONS? What is the difference?

> 
> > > +#define KEY_INTERACTIVE                      0x2ea   /* Jump to interactive system/menu/item */

How is this different from KEY_MENU?

> > > +#define KEY_MIC_INPUT                        0x2eb   /* Trigger MIC input/listen mode */
> >
> > KEY_MICMUTE?
> 
> This button doesn't mute the mic, in fact it does the opposite. The
> mic is off until you press this button, thus triggering MIC
> input/listen mode and allowing the user to speak his commands. It
> automatically shuts off after X seconds of silence.

KEY_VOICECOMMAND then.

> 
> > > +#define KEY_SCREEN_INPUT             0x2ec   /* Open on-screen input system */
> >
> > KEY_SWITCHVIDEOMODE?
> 
> KEY_SWITCHVIDEOMODE is used for "Cycle between available video outputs
> (Monitor/LCD/TV-out/etc) ". This is poorly labeled in my opinion and
> should've been called KEY_SWITCHVIDEOOUTPUT or something similar.
> "Video mode" typically refers to something entirely different - how
> video is presented on the display, not what physical display you're
> using.

It normally controls not only what devices are used for output, but
switches between mirror/extend display modes.

> KEY_SCREEN_INPUT is used to bring up things like an on-screen
> keyboard or other on-onscreen user input method.

We already have KEY_ONSCREEN_KEYBOARD.

> 
> > > +#define KEY_SYSTEM_MENU                      0x2ed   /* Open systems menu/display */
> >
> > KEY_MENU?
> 
> Systems menus as pertains to DVB. KEY_MENU is generic and having only
> one `menu` option is problematic when you have different types of
> menus which aren't accessible from each other.

We have KEY_MENU/KEY_CONTEXT_MENU/KEY_ROOT_MENU/KEY_MEDIA_TOP_MENU.
Are you sure we need another one?

> 
> > > +#define KEY_SERVICES                 0x2ee   /* Access services */
> > > +#define KEY_DISPLAY_FORMAT           0x2ef   /* Cycle display formats */
> >
> > KEY_CONTEXT_MENU?
> 
> KEY_DISPLAY_FORMAT doesn't open any menus and is used to cycle through
> how video is displayed on-screen to the user; full, zoomed,
> letterboxed, stretched, etc. KEY_CONTEXT_MENU would be for something
> like bringing up a playback menu where you'd set things like
> upscaling, deinterlacing, audio mixdown/mixup, etc.

KEY_ASPECT_RATIO (formerly KEY_SCREEN).

> 
> > > +#define KEY_PIP                              0x2f0   /* Toggle Picture-in-Picture on/off */
> > > +#define KEY_PIP_SWAP                 0x2f1   /* Swap contents between main view and PIP window */
> > > +#define KEY_PIP_POSITION             0x2f2   /* Cycle PIP window position */
> > > +
> > >  /* We avoid low common keys in module aliases so they don't get huge. */
> > >  #define KEY_MIN_INTERESTING  KEY_MUTE
> > >  #define KEY_MAX                      0x2ff
> >
> 
> Hopefully that makes things more clear. This patch helps users map
> common (media/htpc/stb) remote control buttons directly to their real
> functions as opposed to mapping them to some random unrelated & unused
> event, which can be both confusing and problematic on systems where
> both remote controls and say bluetooth keyboards are used.

It would be great if you provided references to HID Usage Tables for the
new keycodes you are adding, which should help further clarify the
meaning of keycode. For example, even with the comment, it is not clear
what "Access services" means.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Input: Add missing event codes for common IR remote buttons
  2019-01-19  8:52     ` Dmitry Torokhov
@ 2019-01-23  7:50       ` VDR User
  2019-01-23 10:18         ` Bastien Nocera
  2019-01-24  8:37         ` Dmitry Torokhov
  0 siblings, 2 replies; 10+ messages in thread
From: VDR User @ 2019-01-23  7:50 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Bastien Nocera, linux-input, Sean Young, mchehab+samsung,
	mailing list: linux-media

Hi Dmitry, thanks for replying. Notes follow..

> > > > +/* Remote control buttons found across provider & universal remotes */
> > > > +#define KEY_LIVE_TV                  0x2e8   /* Jump to live tv viewing */
> > >
> > > KEY_TV?
> >
> > KEY_TV selects TV as a *input source* the same as KEY_VCR, KEY_SAT,
> > KEY_CD, etc. whereas KEY_LIVE_TV jumps directly to live tv as opposed
> > to local/networked media playback, dvr playback, etc.
>
> I do not quite grasp the distinction. KEY_TV to select and play
> broadcast TV, KEY_TV2 to switch TV input to cable.

Another example is when you have a remote that controls multiple
devices, KEY_TV would select the TV itself and become the target for
the remote until a different source is selected. This happens without
any change to the playback source. KEY_LIVE_TV would jumps directly to
live tv playback and doesn't touch devices at all.

> > > > +#define KEY_OPTIONS                  0x2e9   /* Jump to options */
> > >
> > > KEY_OPTION?
> >
> > Software vs. media playback options.
>
> This seems application control key. Do you really need both KEY_OPTION
> and KEY_OPTIONS? What is the difference?

Another example for this is device vs. application options.

> > > > +#define KEY_INTERACTIVE                      0x2ea   /* Jump to interactive system/menu/item */
>
> How is this different from KEY_MENU?

The first thing to mention is that traditional broadcast tv and the
internet have been converging so now you have tv with two-way
communication. Interactive tv can be anything from a traditional
broadcast that you can submit ratings for, real time comments, etc.
KEY_INTERACTIVE is specific to when you're interacting or engaging the
content. Rather than having the users inconveniently navigate through
a menu system, providers often have a dedicated button that jumps
directly to the interactive features.

> > > > +#define KEY_MIC_INPUT                        0x2eb   /* Trigger MIC input/listen mode */
> > >
> > > KEY_MICMUTE?
> >
> > This button doesn't mute the mic, in fact it does the opposite. The
> > mic is off until you press this button, thus triggering MIC
> > input/listen mode and allowing the user to speak his commands. It
> > automatically shuts off after X seconds of silence.
>
> KEY_VOICECOMMAND then.

Somehow I missed KEY_VOICECOMMAND when looking over the list for
suitable existing defines. With KEY_VOICECOMMAND, KEY_MIC_INPUT is not
needed.

> > KEY_SWITCHVIDEOMODE is used for "Cycle between available video outputs
> > (Monitor/LCD/TV-out/etc) ". This is poorly labeled in my opinion and
> > should've been called KEY_SWITCHVIDEOOUTPUT or something similar.
> > "Video mode" typically refers to something entirely different - how
> > video is presented on the display, not what physical display you're
> > using.
>
> It normally controls not only what devices are used for output, but
> switches between mirror/extend display modes.

That actually muddies the waters further then. It makes sense to
couple toggling single/multiple output devices with output device
selection, but neither of those things have to do with the video
display modes. These are two distinct things; the physical device(s)
that act simply as a means to display video, and display modes that
refer to how the video frames are drawn - what they actually look
like.

> > KEY_SCREEN_INPUT is used to bring up things like an on-screen
> > keyboard or other on-onscreen user input method.
>
> We already have KEY_ONSCREEN_KEYBOARD.
>
> >
> > > > +#define KEY_SYSTEM_MENU                      0x2ed   /* Open systems menu/display */
> > >
> > > KEY_MENU?
> >
> > Systems menus as pertains to DVB. KEY_MENU is generic and having only
> > one `menu` option is problematic when you have different types of
> > menus which aren't accessible from each other.
>
> We have KEY_MENU/KEY_CONTEXT_MENU/KEY_ROOT_MENU/KEY_MEDIA_TOP_MENU.
> Are you sure we need another one?

There are multiple MENU keys I assume for clarity purposes and to give
some kind of relation between the key definition and the action/event
that occurs when you use it. I would say it's more a matter of
convenience rather that need, similar to KEY_ROOT_MENU &
KEY_MEDIA_TOP_MENU; It's not a necessity that these two exist, but
they do out of convenience. You could still make things work if one of
them vanished.

> > > > +#define KEY_SERVICES                 0x2ee   /* Access services */
> > > > +#define KEY_DISPLAY_FORMAT           0x2ef   /* Cycle display formats */
> > >
> > > KEY_CONTEXT_MENU?
> >
> > KEY_DISPLAY_FORMAT doesn't open any menus and is used to cycle through
> > how video is displayed on-screen to the user; full, zoomed,
> > letterboxed, stretched, etc. KEY_CONTEXT_MENU would be for something
> > like bringing up a playback menu where you'd set things like
> > upscaling, deinterlacing, audio mixdown/mixup, etc.
>
> KEY_ASPECT_RATIO (formerly KEY_SCREEN).

Physical displays have a single set aspect ratio (W/H). Images have
their own aspect ratios. When the AR of the video to be display and
the display itself are mismatched, you have to do something
(letterbox, pillarbox, windowbox) to the video to maintain the correct
video aspect ratio. You can't change the displays AR, and you aren't
changing the videos AR so using KEY_ASPECT_RATIO makes no sense. AR
isn't being touched/altered/manipulated, but how the video is being
displayed is. Stretching and filling to match the display AR alters
the video AR so there is makes sense, but then zooming may not. So,
since "aspect ratio" kind of makes sense in a couple cases, and makes
no sense in the rest, the more suitable KEY_DISPLAY_FORMAT is my
suggestion.

> > > > +#define KEY_PIP                              0x2f0   /* Toggle Picture-in-Picture on/off */
> > > > +#define KEY_PIP_SWAP                 0x2f1   /* Swap contents between main view and PIP window */
> > > > +#define KEY_PIP_POSITION             0x2f2   /* Cycle PIP window position */
> > > > +
> > > >  /* We avoid low common keys in module aliases so they don't get huge. */
> > > >  #define KEY_MIN_INTERESTING  KEY_MUTE
> > > >  #define KEY_MAX                      0x2ff
> > >
> >
> > Hopefully that makes things more clear. This patch helps users map
> > common (media/htpc/stb) remote control buttons directly to their real
> > functions as opposed to mapping them to some random unrelated & unused
> > event, which can be both confusing and problematic on systems where
> > both remote controls and say bluetooth keyboards are used.
>
> It would be great if you provided references to HID Usage Tables for the
> new keycodes you are adding, which should help further clarify the
> meaning of keycode. For example, even with the comment, it is not clear
> what "Access services" means.

My apologies for not including HID Usage Table references. I didn't
know to include it and not really familiar with the tables. This stuff
applies to common but specific setups/usage and it's important to
remember not everyone may be familiar with it. I'll look into it in
the morning and hopefully it will provide easier clarity in future
posts. Thanks again for your reply & feedback!

Best regards,
Derek

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Input: Add missing event codes for common IR remote buttons
  2019-01-23  7:50       ` VDR User
@ 2019-01-23 10:18         ` Bastien Nocera
  2019-01-23 16:07           ` VDR User
  2019-01-24  8:37         ` Dmitry Torokhov
  1 sibling, 1 reply; 10+ messages in thread
From: Bastien Nocera @ 2019-01-23 10:18 UTC (permalink / raw)
  To: VDR User, Dmitry Torokhov
  Cc: linux-input, Sean Young, mchehab+samsung, mailing list: linux-media

On Tue, 2019-01-22 at 23:50 -0800, VDR User wrote:
> > > KEY_SCREEN_INPUT is used to bring up things like an on-screen
> > > keyboard or other on-onscreen user input method.
> > 
> > We already have KEY_ONSCREEN_KEYBOARD.
> > 
> > > > > +#define KEY_SYSTEM_MENU                      0x2ed   /* Open
> > > > > systems menu/display */
> > > > 
> > > > KEY_MENU?
> > > 
> > > Systems menus as pertains to DVB. KEY_MENU is generic and having
> > > only
> > > one `menu` option is problematic when you have different types of
> > > menus which aren't accessible from each other.
> > 
> > We have KEY_MENU/KEY_CONTEXT_MENU/KEY_ROOT_MENU/KEY_MEDIA_TOP_MENU.
> > Are you sure we need another one?
> 
> There are multiple MENU keys I assume for clarity purposes and to
> give
> some kind of relation between the key definition and the action/event
> that occurs when you use it. I would say it's more a matter of
> convenience rather that need, similar to KEY_ROOT_MENU &
> KEY_MEDIA_TOP_MENU; It's not a necessity that these two exist, but
> they do out of convenience. You could still make things work if one
> of
> them vanished.

Those 2 keys were added because they were present on DVD player remotes
nearly 20 years ago ("Root menu" vs. "Top Menu"), and do different
things.

Dmitry, the difference between:
#define KEY_MENU                139     /* Menu (show menu) */
and:
#define KEY_CONTEXT_MENU        0x1b6   /* GenDesc - system context
menu */
isn't super clear to me. The first one is used for contextual menu on
keyboards (the menu key added since Windows 95), does KEY_CONTEXT_MENU
do the same thing?

> > > > > +#define KEY_SERVICES                 0x2ee   /* Access
> > > > > services */
> > > > > +#define KEY_DISPLAY_FORMAT           0x2ef   /* Cycle
> > > > > display formats */
> > > > 
> > > > KEY_CONTEXT_MENU?
> > > 
> > > KEY_DISPLAY_FORMAT doesn't open any menus and is used to cycle
> > > through
> > > how video is displayed on-screen to the user; full, zoomed,
> > > letterboxed, stretched, etc. KEY_CONTEXT_MENU would be for
> > > something
> > > like bringing up a playback menu where you'd set things like
> > > upscaling, deinterlacing, audio mixdown/mixup, etc.
> > 
> > KEY_ASPECT_RATIO (formerly KEY_SCREEN).
> 
> Physical displays have a single set aspect ratio (W/H). Images have
> their own aspect ratios. When the AR of the video to be display and
> the display itself are mismatched, you have to do something
> (letterbox, pillarbox, windowbox) to the video to maintain the
> correct
> video aspect ratio. You can't change the displays AR, and you aren't
> changing the videos AR so using KEY_ASPECT_RATIO makes no sense. AR
> isn't being touched/altered/manipulated, but how the video is being
> displayed is. Stretching and filling to match the display AR alters
> the video AR so there is makes sense, but then zooming may not. So,
> since "aspect ratio" kind of makes sense in a couple cases, and makes
> no sense in the rest, the more suitable KEY_DISPLAY_FORMAT is my
> suggestion.

The "Aspect Ratio" or "Ratio" key on loads of remotes have been doing
this exact thing since the days of the first 16:9/cinema displays. I
really don't think you need a new key here.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Input: Add missing event codes for common IR remote buttons
  2019-01-23 10:18         ` Bastien Nocera
@ 2019-01-23 16:07           ` VDR User
  0 siblings, 0 replies; 10+ messages in thread
From: VDR User @ 2019-01-23 16:07 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: Dmitry Torokhov, linux-input, Sean Young, mchehab+samsung,
	mailing list: linux-media

Hi Bastian,

> > There are multiple MENU keys I assume for clarity purposes and to
> > give
> > some kind of relation between the key definition and the action/event
> > that occurs when you use it. I would say it's more a matter of
> > convenience rather that need, similar to KEY_ROOT_MENU &
> > KEY_MEDIA_TOP_MENU; It's not a necessity that these two exist, but
> > they do out of convenience. You could still make things work if one
> > of
> > them vanished.
>
> Those 2 keys were added because they were present on DVD player remotes
> nearly 20 years ago ("Root menu" vs. "Top Menu"), and do different
> things.

That's true as it pertains to DVD players, but usage has changed over
time. These days it's common to have pc-based alternatives to
standalone players. Beyond that, pc-based setups that replace multiple
standalone devices. I personally use `root menu` to jump to the root
dir of my lan in file browsing mode and `top menu` to jump out to the
top-most menu of the playback menus. I mentioned neither of these _has
to_ exist but do for the sake of convenience. That's because the end
result in both cases could be achieved by pressing `back` X number of
times. But that's slow & inconvenient.

> > > > KEY_DISPLAY_FORMAT doesn't open any menus and is used to cycle
> > > > through
> > > > how video is displayed on-screen to the user; full, zoomed,
> > > > letterboxed, stretched, etc. KEY_CONTEXT_MENU would be for
> > > > something
> > > > like bringing up a playback menu where you'd set things like
> > > > upscaling, deinterlacing, audio mixdown/mixup, etc.
> > >
> > > KEY_ASPECT_RATIO (formerly KEY_SCREEN).
> >
> > Physical displays have a single set aspect ratio (W/H). Images have
> > their own aspect ratios. When the AR of the video to be display and
> > the display itself are mismatched, you have to do something
> > (letterbox, pillarbox, windowbox) to the video to maintain the
> > correct
> > video aspect ratio. You can't change the displays AR, and you aren't
> > changing the videos AR so using KEY_ASPECT_RATIO makes no sense. AR
> > isn't being touched/altered/manipulated, but how the video is being
> > displayed is. Stretching and filling to match the display AR alters
> > the video AR so there is makes sense, but then zooming may not. So,
> > since "aspect ratio" kind of makes sense in a couple cases, and makes
> > no sense in the rest, the more suitable KEY_DISPLAY_FORMAT is my
> > suggestion.
>
> The "Aspect Ratio" or "Ratio" key on loads of remotes have been doing
> this exact thing since the days of the first 16:9/cinema displays. I
> really don't think you need a new key here.

You can throw "Display" buttons and buttons with simply an icon
implying display manipulation in there as well. I've explained why
KEY_ASPECT_RATIO simply doesn't apply, and why KEY_DISPLAY_FORMAT
does. If people are against adding KEY_DISPLAY_FORMAT, then
KEY_ASPECT_RATIO should be renamed again & as such to properly reflect
the intended action. I suspect whomever started labeling this as
"aspect" or "aspect ratio" on remotes was some hardware designer that
didn't know how to accurately explain what the button does and took
his best (poor) guess.

Generally speaking, I think it's important to keep key definitions
close to what the intended action/result is. When it comes to pc-based
solutions, they are taking on more simultaneous roles and have to
cover a wider range of stuff. It's common these days to find people
using small boxes which perform local media functions & playback,
recording, streaming, interactive tv, web browsing access, etc. The
whole reason I made these suggestions was because I found I was
assigning keys which either didn't properly describe the event, or
were completely unrelated to it, and then having to change the
definition in software to match the actual use. Picture-In-Picture is
a great example of this. There's no defines that cover any of the PIP
stuff so you're forced to use something totally unrelated
(KEY_WHATEVER) and then alter software so when you press KEY_WHATEVER,
what you really mean is <some PIP function here>.

Best regards,
Derek

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Input: Add missing event codes for common IR remote buttons
  2019-01-23  7:50       ` VDR User
  2019-01-23 10:18         ` Bastien Nocera
@ 2019-01-24  8:37         ` Dmitry Torokhov
  2019-01-24 16:21           ` VDR User
  1 sibling, 1 reply; 10+ messages in thread
From: Dmitry Torokhov @ 2019-01-24  8:37 UTC (permalink / raw)
  To: VDR User
  Cc: Bastien Nocera, linux-input, Sean Young, mchehab+samsung,
	mailing list: linux-media

On Tue, Jan 22, 2019 at 11:50:50PM -0800, VDR User wrote:
> > >
> > > KEY_DISPLAY_FORMAT doesn't open any menus and is used to cycle through
> > > how video is displayed on-screen to the user; full, zoomed,
> > > letterboxed, stretched, etc. KEY_CONTEXT_MENU would be for something
> > > like bringing up a playback menu where you'd set things like
> > > upscaling, deinterlacing, audio mixdown/mixup, etc.
> >
> > KEY_ASPECT_RATIO (formerly KEY_SCREEN).
> 
> Physical displays have a single set aspect ratio (W/H). Images have
> their own aspect ratios. When the AR of the video to be display and
> the display itself are mismatched, you have to do something
> (letterbox, pillarbox, windowbox) to the video to maintain the correct
> video aspect ratio. You can't change the displays AR, and you aren't
> changing the videos AR so using KEY_ASPECT_RATIO makes no sense. AR
> isn't being touched/altered/manipulated, but how the video is being
> displayed is. Stretching and filling to match the display AR alters
> the video AR so there is makes sense, but then zooming may not. So,
> since "aspect ratio" kind of makes sense in a couple cases, and makes
> no sense in the rest, the more suitable KEY_DISPLAY_FORMAT is my
> suggestion.

No, we will not be renaming this key. We try to have parity with the
HUT, which has the following to say:

"Aspect OSC - Selects the next available supported aspect ratio option
on a device which outputs or displays video.  For example, common
aspect ratio options are 4:3 (standard definition), 16:9 (often used
to stretch a standard definition source signal to a 16:9 video screen),
letter-box and anamorphic widescreen.The order in which the aspect
ratios are selected is implementation specific."

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Input: Add missing event codes for common IR remote buttons
  2019-01-24  8:37         ` Dmitry Torokhov
@ 2019-01-24 16:21           ` VDR User
  0 siblings, 0 replies; 10+ messages in thread
From: VDR User @ 2019-01-24 16:21 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Bastien Nocera, linux-input, Sean Young, mchehab+samsung,
	mailing list: linux-media

> > > KEY_ASPECT_RATIO (formerly KEY_SCREEN).
> >
> > Physical displays have a single set aspect ratio (W/H). Images have
> > their own aspect ratios. When the AR of the video to be display and
> > the display itself are mismatched, you have to do something
> > (letterbox, pillarbox, windowbox) to the video to maintain the correct
> > video aspect ratio. You can't change the displays AR, and you aren't
> > changing the videos AR so using KEY_ASPECT_RATIO makes no sense. AR
> > isn't being touched/altered/manipulated, but how the video is being
> > displayed is. Stretching and filling to match the display AR alters
> > the video AR so there is makes sense, but then zooming may not. So,
> > since "aspect ratio" kind of makes sense in a couple cases, and makes
> > no sense in the rest, the more suitable KEY_DISPLAY_FORMAT is my
> > suggestion.
>
> No, we will not be renaming this key. We try to have parity with the
> HUT, which has the following to say:
>
> "Aspect OSC - Selects the next available supported aspect ratio option
> on a device which outputs or displays video.  For example, common
> aspect ratio options are 4:3 (standard definition), 16:9 (often used
> to stretch a standard definition source signal to a 16:9 video screen),
> letter-box and anamorphic widescreen.The order in which the aspect
> ratios are selected is implementation specific."

Hi Dmitry,

The suggestion to rename KEY_ASPECT_RATIO was a `last resort` in the
event people are against a more suitable and accurate key to what
happens to the actual content. The root issue is in not making any
distinction between content AR, frame AR, and display AR. As
previously described, letterbox/pillarbox/windowbox is used to
*maintain* the correct AR, not alter it. Stretching may or may not
alter AR, zooming may or may not alter AR. There are more scenarios
where AR is unaffected than otherwise so more often KEY_ASPECT_RATIO
makes no sense than does. However, in all cases (except where content
& display match and you wouldn't use this key) how the frames are
being displayed *is* altered, which is where KEY_DISPLAY_FORMAT comes
from.

Best regards,
Derek

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-01-24 16:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-03 14:55 [PATCH v2] Input: Add missing event codes for common IR remote buttons Derek Kelly
2018-11-05 20:53 ` Sean Young
2018-11-13 13:04 ` Bastien Nocera
2018-11-13 16:20   ` VDR User
2019-01-19  8:52     ` Dmitry Torokhov
2019-01-23  7:50       ` VDR User
2019-01-23 10:18         ` Bastien Nocera
2019-01-23 16:07           ` VDR User
2019-01-24  8:37         ` Dmitry Torokhov
2019-01-24 16:21           ` VDR User

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.