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

[Alexey Sokolov]

12.08.2012 23:33, Rémi Denis-Courmont пишет:
> Le dimanche 12 août 2012 15:33:59 Alexey Sokolov, vous avez écrit :
>> 12.08.2012 19:29, Rémi Denis-Courmont пишет:
>>> Le dimanche 12 août 2012 13:49:37 Alexey Sokolov, vous avez écrit :
>>>> 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?
>>> Why use a reference count? This is just plain (small) data.
>>>
>>> Way overkill.
>> To not to need to maintain validness of the pointer ourself.
> That sentence does not even make sense.
>
> In any case, whether you have reference counting, copying on read or copying 
> on write, you always need to take care of the life cycle of your reference.
>
> In other words, reference counting is absolutely useless cruft unless there is 
> a point in sharing the resource. Here, there is *absolutely* no point.
>
>> If it's so small data, why not to store it inside the media_player
>> completely instead?
> The data is stored in the aout, where it belongs.
>
Then why the comment says that 

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().

?
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);

-- 
Best regards,
Alexey "DarthGandalf" Sokolov


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 900 bytes
Desc: OpenPGP digital signature
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120823/1b122fac/attachment.sig>