Re: [PATCH v2] Input: Add missing event codes for common IR remote buttons

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux