<br><br><div class="gmail_quote">On Wed, Nov 11, 2009 at 11:44 AM, <span dir="ltr"><<a href="mailto:y.brehon@qiplay.com">y.brehon@qiplay.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Please find attached a corrected version, following your recommendations.<br>
Please feel free to comment.<br></blockquote><div><br>Thansk for following advise and help of the videolan developers. I hope you will continue to do so, because the works is not yet finished for this feature. Here are my comments:<br>
<br>1) use function names that tell what they do <br><pre>+<br>+// Make a copy of an NPVariant.<br>+NPVariant saveNPVariant(const NPVariant& original)<br></pre>Use a more instructive name. It is meant to copy a NPVariant, then name it eg: copyNPVariant() iso saveNPVariant(). 'save' indicates more permanent storage and not making a copy.<br>
<br><pre>+void eventFct(const libvlc_event_t* event, void *param)<br>+{<br></pre>Use eg; event_callback() iso eventFct(). Callback is more descriptive.<br><br>2) use ID_event_* names here iso ID_root_*. You are extending API's for eventObj not for rootObj.<br>
<pre>+enum LibvlcEventNPObjectMethodIds<br>+{<br>+ ID_root_addListener,<br>+ ID_root_removeListeners,<br>+};<br></pre>3) You methods/lists of type vector, but nowhere in the code you use that fact to your benefit.<br>
<pre> class VlcPlugin<br> {<br> public:<br>@@ -197,6 +305,15 @@ public:<br> <br> bool player_has_vout( libvlc_exception_t * );<br> <br>+<br>+ // Events related members<br>+ std::vector<EventListener*> eventListeners; // List of registered listerners.<br>
+ std::vector<const NPUTF8*> eventList; // List of event sent by VLC that must be returned to JS.<br>+ eventtypes_bitmap_t eventToCatch;<br>+ pthread_mutex_t mutex;<br>+<br>+ static bool canUseEventListener();<br>
+<br> private:<br> bool playlist_select(int,libvlc_exception_t *);<br> void set_player_window( libvlc_exception_t * );<br></pre><br>Kind regards,<br><br>Jean-Paul Saman<br></div></div>