Hello,<br><br><div class="gmail_quote">On 10 July 2013 20:05, Rémi Denis-Courmont <span dir="ltr"><<a href="mailto:remi@remlab.net" target="_blank">remi@remlab.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Le mercredi 10 juillet 2013 21:54:18, Mark Lee a écrit :<br>
> +/*************************************************************************<br>
> **** + * libvlc_audio_equalizer_set_amp_at_index : Set the amplification<br>
<div class="im">> value for an equalizer band +<br>
> **************************************************************************<br>
</div>> ***/ +int libvlc_audio_equalizer_set_amp_at_index( libvlc_equalizer_t<br>
<div class="im">> *p_equalizer, float f_amp, unsigned u_band ) +{<br>
> +    if ( !p_equalizer )<br>
> +        return -1;<br>
<br>
</div>Whenever you accept NULL, please document it. Alternatively, do not accept<br>
NULL.<br></blockquote><div><br>OK, so I can add something in the header file method documentation explicitly<br>stating that passing in NULL will return an error.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="im"><br>
> +<br>
> +    if ( u_band >= EQZ_BANDS_MAX )<br>
> +        return -1;<br>
> +<br>
> +    if ( f_amp < -20.0f )<br>
> +        f_amp = -20.0f;<br>
> +    else if ( f_amp > 20.0f )<br>
> +        f_amp = 20.0f;<br>
> +<br>
> +    p_equalizer->f_amp[ u_band ] = f_amp;<br>
> +    return 0;<br>
> +}<br>
> +<br>
</div><div><div class="h5">> diff --git a/lib/media_player.c b/lib/media_player.c<br>
> index 376159f..bb43a82 100644<br>
> --- a/lib/media_player.c<br>
> +++ b/lib/media_player.c<br>
> @@ -33,6 +33,7 @@<br>
>  #include <vlc_demux.h><br>
>  #include <vlc_input.h><br>
>  #include <vlc_vout.h><br>
> +#include <vlc_aout.h><br>
>  #include <vlc_keys.h><br>
><br>
>  #include "libvlc_internal.h"<br>
> @@ -462,6 +463,10 @@ libvlc_media_player_new( libvlc_instance_t *instance )<br>
>      var_Create (mp, "video-title-position", VLC_VAR_INTEGER);<br>
>      var_Create (mp, "video-title-timeout", VLC_VAR_INTEGER);<br>
><br>
> +    /* Equalizer */<br>
> +    var_Create (mp, "equalizer-preamp", VLC_VAR_FLOAT);<br>
> +    var_Create (mp, "equalizer-bands", VLC_VAR_STRING);<br>
> +<br>
>      mp->p_md = NULL;<br>
>      mp->state = libvlc_NothingSpecial;<br>
>      mp->p_libvlc_instance = instance;<br>
> @@ -1404,3 +1409,46 @@ void libvlc_media_player_set_video_title_display(<br>
> libvlc_media_player_t *p_mi, l var_SetBool( p_mi, "video-title-show",<br>
> false );<br>
>      }<br>
>  }<br>
> +<br>
> +int libvlc_media_player_set_equalizer( libvlc_media_player_t *p_mi,<br>
> libvlc_equalizer_t *p_equalizer ) +{<br>
> +    float f_preamp;<br>
> +    char *psz_bands;<br>
> +<br>
> +    if ( p_equalizer )<br>
> +    {<br>
> +        f_preamp = p_equalizer->f_preamp;<br>
> +        psz_bands = NULL;<br>
> +        for ( int i = 0; i < EQZ_BANDS_MAX; i++ )<br>
> +        {<br>
> +            char *psz;<br>
> +            if ( asprintf( &psz, "%s %.07f", psz_bands ? psz_bands : "",<br>
> p_equalizer ? p_equalizer->f_amp[i] : 0.f ) == -1 )<br>
> +            {<br>
> +                free( psz_bands );<br>
> +                return -1;<br>
> +            }<br>
<br>
</div></div>How large can this get? Pre-allocating the buffer would be faster and simpler,<br>
especially on the stack.<br>
<div><div class="h5"><br></div></div></blockquote><div><br>It's usually 100 bytes or so.<br><br>I thought I copied/adapted that from some code elsewhere in vlc, I can rework it.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div class="h5">
> +            free( psz_bands );<br>
> +            psz_bands = psz;<br>
> +        }<br>
> +    }<br>
> +    else<br>
> +    {<br>
> +        f_preamp = 0.f;<br>
> +        psz_bands = NULL;<br>
> +    }<br>
> +<br>
> +    var_SetFloat( p_mi, "equalizer-preamp", f_preamp );<br>
> +    var_SetString( p_mi, "equalizer-bands", psz_bands );<br>
> +<br>
> +    audio_output_t *p_aout = input_resource_HoldAout(<br>
> p_mi->input.p_resource ); +    if ( p_aout )<br>
> +    {<br>
> +        var_SetFloat( p_aout, "equalizer-preamp", f_preamp );<br>
> +        var_SetString( p_aout, "equalizer-bands", psz_bands );<br>
> +<br>
> +        vlc_object_release( p_aout );<br>
> +    }<br>
> +<br>
> +    free( psz_bands );<br>
> +    return 0;<br>
> +}<br>
</div></div><div class="im">> diff --git a/src/audio_output/filters.c b/src/audio_output/filters.c<br>
> index 41b23b4..ea451a3 100644<br>
> --- a/src/audio_output/filters.c<br>
> +++ b/src/audio_output/filters.c<br>
> @@ -314,8 +314,7 @@ static int EqualizerCallback (vlc_object_t *obj, const<br>
> char *var, void *data)<br>
>  {<br>
>      const char *val = newval.psz_string;<br>
> -<br>
> -    if (*val)<br>
> +    if (!strcmp("equalizer", var) && *val)<br>
<br>
</div>Tautology.<br>
</blockquote><div> <br>Sorry, I don't understand this.<br><br>I am reusing this existing EqualizerCallback to newly track the "equalizer-preamp"<br>variable (currently it is used only for the "equalizer" variable, which is triggered by<br>
some other independent mechanism to create and set an "equalizer-preset"<br>variable).<br><br>I am reusing the callback because it already has the code to enable/disable the <br>equalizer filter, but it also has another code path I do not want to execute (originally<br>
I had it as a separate callback).<br><br>So, the value of "var" can now be "equalizer" or "equalizer-bands", and the value<br>of "*val" legitimately can either be NULL or not-NULL.<br>
<br>Apologies if I've misunderstood, would you please elaborate?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
>      {<br>
>          var_Create (obj, "equalizer-preset", VLC_VAR_STRING);<br>
>          var_SetString (obj, "equalizer-preset", val);<br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Rémi Denis-Courmont<br>
<a href="http://www.remlab.net/" target="_blank">http://www.remlab.net/</a><br>
</font></span></blockquote></div><br><br><a href="http://apricasoftware.co.uk" target="_blank"></a>