[vlc-devel] [PATCH 1/2] Add new API to libvlc for persistent equalizer settings.

[Mark Lee]

Hello...

On 12 August 2012 11:49, Alexey Sokolov <alexey at asokolov.org> wrote:

> Hello.
>
> 12.08.2012 15:30, Mark Lee пишет:
> > +/**
> > + * Create a new default equalizer, with all frequency values zeroed.
> > + *
> > + * The new equalizer can subsequently be associated with a media player
> by using
> > + * libvlc_media_player_set_equalizer().
> > + *
> > + * When the application is finished with a particular equalizer
> instance it should
> > + * disassociate it from the media player by invoking
> libvlc_media_player_set_equalizer(NULL)
> > + * and then free it by using libvlc_audio_equalizer_release().
> > + *
>
> Why not to use reference count, as other libvlc objects do? So no need
> to manually preserve the pointer to the equalizer which is just
> associated with a player.
> E.g. you're free to call libvlc_media_release(p_media) just after adding
> this media to media_player.
>
>
I did think about that. The original discussion that I referenced on the
mailing list from way-back-when suggested reference counting would not be
needed for the equalizer.

I think the handling of libvlc_media_t is a fair analogy.


> > + * It is the responsibility of the application itself to deal wth the
> lifecycle management
> > + * of equalizer instances, and not the responsibilty of libvlc.
> > + *
> > + * \return opaque equalizer handle, or NULL on error
> > + * \version LibVLC 2.1.0 or later
> > + */
> > +LIBVLC_API libvlc_equalizer_t *libvlc_audio_equalizer_new( void );
> > +
>
> I'd make it libvlc_audio_equalizer_new(libvlc_instance_t*) for more
> consistency with other libvlc objects
>
> That instance parameter would not be used for anything unless reference
counting is definitely needed.

So is there a consensus that reference counting is definitely needed? I am
happy to make those changes, but the reference counting change is quite
significant so I would like some consensus on this point first.


> > +/**
> > + * Create a new equalizer, with initial frequency values copied from an
> existing
> > + * preset.
> > + *
> > + * The new equalizer can subsequently be associated with a media player
> by using
> > + * libvlc_media_player_set_equalizer().
> > + *
> > + * When the application is finished with a particular equalizer
> instance it should
> > + * disassociate it from the media player by invoking
> libvlc_media_player_set_equalizer(NULL)
> > + * and then free it by using libvlc_audio_equalizer_release().
> > + *
> > + * It is the responsibility of the application itself to deal with the
> lifecycle management
> > + * of equalizer instances, and not the responsibilty of libvlc.
> > + *
> > + * \param i_index index of the preset, counting from zero
> > + * \return opaque equalizer handle, or NULL on error
> > + * \version LibVLC 2.1.0 or later
> > + */
> > +LIBVLC_API libvlc_equalizer_t *libvlc_audio_equalizer_new_from_preset(
> int i_index );
> > +
>
> Same here
>
> --
> Best regards,
> Alexey "DarthGandalf" Sokolov
>
> Thanks for your comments.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120812/8c99987f/attachment.html>