[vlc-devel] [PATCH 1/2] Extend libvlc Media Player API for DVD menu navigation

[Arnaud VALLAT]

On Wed, Sep 8, 2010 at 9:57 AM, Arnaud VALLAT <rno.rno at gmail.com> wrote:
> Hello,
>
> 2010/9/7 Rémi Denis-Courmont <remi at remlab.net>:
>>
>> On Tue,  7 Sep 2010 14:41:36 +0200, Arnaud Vallat <rno.rno at gmail.com>
>> wrote:
>>> +    switch (navigate)
>>> +      {
>>> +      case libvlc_navigate_up:
>>> +     i_key = vlc_GetActionId("key-nav-up");
>>> +     break;
>>> +
>>> +      case libvlc_navigate_down:
>>> +     i_key = vlc_GetActionId("key-nav-down");
>>> +     break;
>>> +
>>> +      case libvlc_navigate_left:
>>> +     i_key = vlc_GetActionId("key-nav-left");
>>> +     break;
>>> +
>>> +      case libvlc_navigate_right:
>>> +     i_key = vlc_GetActionId("key-nav-right");
>>> +     break;
>>
>> Oh no. First you convert an integer to a string using a linear search
>> (switch statement).
>> Then you convert the string back to an integer using a binary search
>> (vlc_GetActionId).
>>
>> How about a static constant table that maps your libvlc enum directly to
>> vlc_key? It's much simpler and it is O(1).
>
> First of all: thanks for the review :-)
>
> Yes I can do that but vlc_keys are defined in vlc_keys.h which is not part
> of libvlc headers. However I can write proper function for each navigation
> which would take only libvlc_media_player_t as parameter. And then, those
> functions will directly use vlc_keys value for each direction.

Are you ok with that?

>
> Regards,
>
>>
>> --
>> Rémi Denis-Courmont
>> http://www.remlab.net
>> http://fi.linkedin.com/in/remidenis
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> http://mailman.videolan.org/listinfo/vlc-devel
>>
>
>
>
> --
>
>   "Given enough eyeballs, all bugs are shallow"
>     Eric Steven Raymond
>



-- 

  "Given enough eyeballs, all bugs are shallow"
    Eric Steven Raymond