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

[Mark Lee]

Hello...

On 22 August 2012 19:31, Alexey Sokolov <alexey at asokolov.org> wrote:
[snip]

> If the data is stored in the aout, is the application free to release it
> right after doing libvlc_media_player_set_equalizer(p_mp, p_eq)?
> And only set it to NULL when the application is finished.
> So in this case libvlc_media_player_get_equalizer will return stale
> pointer?
>
> I don't think that requiring users to call
> libvlc_audio_equalizer_release() only when the application is finished
> just to make result of libvlc_media_player_get_equalizer to be
> non-broken, is good.
>
> Maybe just drop libvlc_media_player_get_equalizer then? Or let it create
> new instance of libvlc_equalizer_t, or let it put existing equalizer
> data into the parameter of the function, like "void
> libvlc_media_player_get_equalizer(libvlc_media_player_t*,
> libvlc_equalizer_t* result);
>

I have already been working on a new patch to address this problem.

The only reason I kept that pointer to the equalizer was so it could be
returned back to the application. At the time, that seemed like a
reasonable API to have. Once the equalizer has been set the pointer is no
longer actually used.

For the new patch, I ended up considering these approaches, some are
similar or the same as you suggest:

1. Drop libvlc_media_player_get_equalizer altogether. The media player now
no longer cares about the lifecycle of equalizer instances. It uses the
equalizer to set the values, and that's it.

2. When setting the equalizer, make a copy of the equalizer and store that
copy in the media player. Then, when getting the equalizer another copy is
made and returned - or NULL is returned if there is no equalizer. This
means the media player maintains the lifecycle of its own equalizer
pointer, at the expense of making copies (and requiring the application to
release the copy).

3. Instead of storing the equalizer, or storing a copy of the equalizer,
create and populate a new one on demand. This works, but the code is more
complicated since it must translate the equalizer bands string variable
into an array of floats.

Right now I'm not so sure which way to go, #1 is obviously the simplest,
but #2 is currently my preference.

I'm still mulling it over, any direction or other ideas are welcome.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120822/4db67722/attachment.html>