<div dir="ltr"><div><div>Sorry for top-posting, I'm not specifically replying to one person or another here.<br><br></div>To summarise so far: the original patch that I submitted had the purpose of making the libvlc_media_player_set_video_title_display() API method work according to the API specification. As of right now, it would work correctly for some values of libvlc_position_t, but not for others - this would result in the video title sometimes appearing at the wrong position.<br>
<br></div><div></div>I suppose the original intent of libvlc_position_t was to be a
generic enumeration of "positions" that could be used throughout LibVLC
whenever a position value was needed.<br><div><br>So the follow-up issue arising from this thread I think distils down to what do you want the public API to be? I see these options:<br><br>1. use the public libvlc_position_t enumeration as-is and change the implementation of the API method;<br>
2. change the public libvlc_position_t enumeration to match the constants in include/vlc_subpicture.h;<br>3. change the public libvlc_position_t enumeration to use the constants in include/vlc_subpicture.h;<br></div><div>
4. something else.<br></div><div><br></div><div>For #1, I would envisage that the implementation of the libvlc_media_player_set_video_title_display() method would change to set the value of the "video-title-position" variable by translating from the position enumeration to the corresponding constants defined in "include/vlc_subpicture.h" e.g. in a switch statement. At least with this approach the public LibVLC API values are decoupled from the private subpicture implementation values.<br>
</div><div><br></div><div>For #2, with my initial patch at least the API method will work according to its specification. The patch right now defines those constants directly as integer values that happen to equate to the values of constants defined in "include/vlc_subpicture.h". Whether these are defined directly as integers, or bitmasks or whatever (#3) is a lesser issue since it would be easy for me to change that to however you want.<br>
</div><div></div><div><br></div><div>Instinctively it seems to me like the public position enumeration values should *not* be defined in terms of the private subpicture implementation, since conceivably the position values could be used for something other than subpictures - even though that is not the case right now. So I would prefer #1 I suppose, which if agreed would make the discussion about how to specify those constant values moot.<br>
</div><div><br></div><div>I am happy to prepare another patch if I could please get a bit more guidance.<br></div><div><br></div><div>Regards,<br><br></div><div class="gmail_extra">-M.<br><br><div class="gmail_quote">On 20 May 2014 14:26, Rémi Denis-Courmont <span dir="ltr"><<a href="mailto:remi@remlab.net" target="_blank">remi@remlab.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Le 2014-05-20 20:44, Francois Cartegnie a écrit :<div class=""><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Le 20/05/2014 14:27, Mark Lee a écrit :<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
/* Subpicture region position flags */<br>
#define SUBPICTURE_ALIGN_LEFT 0x1<br>
#define SUBPICTURE_ALIGN_RIGHT 0x2<br>
#define SUBPICTURE_ALIGN_TOP 0x4<br>
#define SUBPICTURE_ALIGN_BOTTOM 0x8<br>
</blockquote>
<br>
Having defines & enums is bad for consistency.<br>
</blockquote>
<br></div>
How so? libc even mixes both, e.g. so that #ifdef works.<div class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Maybe a best way is to split and compose enums:<br>
<br>
typedef enum libvlc_align_t {<br>
libvlc_align_left = 1 << 0,<br>
libvlc_align_right= 1 << 1,<br>
libvlc_align_bottom= 1 << 2,<br>
libvlc_align_top= 1 << 3,<br>
}<br>
<br>
and then compose our second enum based on those values<br>
<br>
<br>
typedef enum libvlc_position_t ...<br>
libvlc_position_left = libvlc_align_left,<br>
libvlc_position_top_left = ( libvlc_align_left|libvlc_align_top ),<br>
</blockquote>
<br></div>
That's even more confusing than either a single enum or a set of #define's IMHO.<span class=""><font color="#888888"><br>
<br>
-- <br>
Rémi Denis-Courmont</font></span><div class=""><div class="h5"><br>
_______________________________________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="https://mailman.videolan.org/listinfo/vlc-devel" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br>Mark Lee<br>mark.lee 'at' <a href="http://mark-lee.com" target="_blank">c</a><a href="http://apricasoftware.co.uk" target="_blank">apricasoftware.co.uk</a>
</div></div>