[vlc-devel] [PATCH] Add marq filter to libvlc

[Cyril MATHE]

Hello,
This is the new patch for marq. One function is added:
libvlc_marq_set_option(libvlc_marq_option_t, int i_val, char *psz_text)
It enables, disables or sets a marq option.

Regards,
	Cyril Mathé



Le lundi 22 juin 2009 à 11:01 -0700, Pierre d'Herbemont a écrit :
> On Mon, Jun 22, 2009 at 7:48 AM, Cyril
> MATHE<cmathe at actech-innovation.com> wrote:
> >> This structure name does not follow the usual libvlc naming scheme.
> >>
> > maybe something like libvlc_video_marq_t ?
> >
> >> Plus, exposing a direct structure is a bit suspicious, especially in
> >> this case where an option is being used to. And it is highly likely
> >> that user will forget to free() the psz_text member of this structure.
> >>
> > Libvlc user have to be aware of that, doesn't he ?
> 
> Yes, that's a point against using that structure.
> 
> >> What about something like:
> >>
> >> libvlc_video_set_marq_text(const char * text);
> >> char * libvlc_video_get_marq_text(const text); // Caller has to free()
> >>
> >> libvlc_video_set_marq_option(libvlc_video_marq_option_t option,
> >> libvlc_video_marq_value_t value);
> >> libvlc_video_get_marq_option(libvlc_video_marq_option_t option,
> >> libvlc_video_marq_value_t value);
> >>
> >> libvlc_video_marq_value_t being a int.
> >>
> >> Pierre.
> >
> > I had done something like this before but fenrir and j-b prefered a
> > solution with a unique structure and two functions. It was :
> > libvlc_video_get_marq() //get marq options
> > libvlc_video_set_marq(b_enable) //enable or disable marq
> > libvlc_video_set_marq_option (libvlc_video_marq_option_t option, int
> > value, char * psz_text) //set a marq option
> 
> I don't agree with J-B and Laurent, a structure seems not appropriate.
> Especially because extension will be harder. But I do agree that first
> prototype was not clear enough. Especially
> libvlc_video_set_marq_option prototype.
> 
> > With your solution there is one function for int options and one for
> > char * options, but in both we have to deal with marq enabling. I don't
> > know if it's really a problem but I noticed it.
> 
> 
> marq enabling would be an option:
> 
> libvlc_video_set_marq_option(libvlc_marq_Enabled, true);
> 
> You could add a new function for it though, if it seems clearer for you.
> 
> After more thinking we could even go further and have:
> 
> libvlc_video_set_marg_option_as_int(libvlc_marq_Blah, num);
> libvlc_video_set_marg_option_as_string(libvlc_marq_Text, "string");
> (And associated getters)
> 
> And to be even more explicit we would have libvlc_marq_string_option_t
> and libvlc_marq_int_option_t.
> 
> It's up to you now.
> 
> Pierre.
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0010-libvlc-API-Add-Marquee-Filter-to-libvlc-in-video.c.patch
Type: application/mbox
Size: 6660 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20090623/40b30271/attachment.mbox>