AudioHumLab / FIRtro

An active loudspeaker xover and eq DIY system, it includes drc and a preamp with a calibrated & loudness iso 226:2003 volume control
GNU General Public License v3.0
6 stars 0 forks source link

web de control, volume slider con decimales #7

Closed Rsantct closed 7 years ago

Rsantct commented 7 years ago

En Debian 9, se observa que el indicador asociado al slider de volumen de la web aparece con decimales y al usarlo los introduce en el cómputo del level del sistema.

amiguelez commented 7 years ago

una ñapa temporal puede ser convertir el valor leído a entero sin decimales, antes de enviarlo al server en la función php.

rripio commented 7 years ago

Ya me funciona la web en todos los dispositivos. Parece que los que tenían la página en caché fallaban. Una limpieza y listo.

Rsantct commented 7 years ago

Ooops no se por qué le puse la etiqueta 'wontfix' a esta issue, se queda con 'bug'

Pregunta: ¿el indicador numérico a la izquierda del slider os muestra un valor con decimales distinto de el valor actual de level? (solo me pasa en Debian 9.1)

rripio commented 7 years ago

No, muestra un valor entero.

2017-08-26 10:51 GMT+02:00 Rsantct notifications@github.com:

Ooops no se porque le puse la etiqueta 'wontfix' a esta issue, se queda con 'bug'

Pregunta: ¿el indicador numérico a la izquierda del slider os muestra un valor con decimales distinto de el valor actual de level?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/AudioHumLab/FIRtro/issues/7#issuecomment-325104371, or mute the thread https://github.com/notifications/unsubscribe-auth/AGkTERkDFECQxWCpFvJ8P5Ik0KCAT3gzks5sb9ydgaJpZM4PCY6L .

-- Roberto Ripio

Rsantct commented 7 years ago

El server proporciona el valor máximo de nivel posible (maxlevel_i) a la web, que se calcula con maxlevel_i = gmax - ref_level_gain - input_gain

Siendo ref_level_gain la ganancia del altavoz declarada en lspk/altavoz/speaker, que puede tener decimales :-) JEJEJE

maxlevel_i será tratado por el código js de gestión del slider, con lo que si tiene decimales al operar en el slider los trasladará al server de vuelta.

El problema desaparece calculando el tope con np.floor: maxlevel_i = np.floor(gmax - ref_level_gain - input_gain)

rripio commented 7 years ago

¿Y por qué el volumen no puede tener decimales? No se ha determinado en ningún sitio que los incrementos en dB tengan que ser enteros. Otra cuestión es que la web maneje solo enteros, pero es algo arbitrario, problema de la web. Un cliente que maneje decimales debería pasar y recibir decimales del server.

Meter un clamp a número entero en el cálculo del server es un error.

Rsantct commented 7 years ago

Es bien lo que comentas. Pero la aritmética de level/gain se computa bien en el server, con decimales a todos los efectos: el punto principal es el headroom tras aplicar las eqs y considerar la ganacia del altavoz que si admite decimales.

De hecho comand level_add 0.1 funciona bien.

Si no consideramos crítico que el volumen de escucha resulte redondeado en saltos de 1 dB, podríamos dejarlo así. No se si los equipos hifi afinan tanto..

Si no, habría que retocar el código javascrís del slider para que no haga un offset en su rango operativo... no se yo si vale la pena, pero si hay dudas e esto, creo que esta sería la opción.

¿Que opináis?

Rsantct commented 7 years ago

ahora, un cliente que maneje decimales funcionará bien, la cosa es que solamente ti tocamos el slider de la web (no los botones), el volumen de escucha será redondeado...

rripio commented 7 years ago

2017-08-26 20:19 GMT+02:00 Rsantct notifications@github.com:

ahora, un cliente que maneje decimales funcionará bien, la cosa es que solamente ti tocamos el slider de la web (no los botones), el volumen de escucha será redondeado...

Si es así vale, es una peculiaridad del cliente. Pero el clamp a entero del headroom lo hace el server ¿no? No hay ningún motivo para eso menos salvar al soldado client. No está bien. El server no puede imponer una cosa así, deben ser los clientes los que digieran los decimales y los transformen a enteros si les sientan mal.

rripio commented 7 years ago

Otra cosa: No está bien que se use la función floor (o ceil, para el caso). Redondear 1.99999 a 1, por ejemplo, es una burrada. El redondeo (que debería hacerse en el cliente) será de medio intervalo. 1.4 se redondea a 1, 1.6 se redondea a 2. Lo normal :-)

rripio commented 7 years ago

Si se deja como ñapa provisional mientras no se toca la web, vale, pero entonces dejamos abierto el issue con la voluntad de cambiarlo en el cliente cuando toque, si te parece.

amiguelez commented 7 years ago

Redondear en la parte web, dentro del código php/java es fácil.

Enviado desde algún lugar

El 26 ago 2017, a las 21:10, Roberto Ripio notifications@github.com escribió:

Si se deja como ñapa provisional mientras no se toca la web, vale, pero entonces dejamos abierto el issue con la voluntad de cambiarlo en el cliente cuando toque, si te parece.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

Rsantct commented 7 years ago

Lo del floor es por ser conservador, maxlevel_i es un tope de volumen de escucha meramente informativo, de uso exclusivo para potes de volumen externos. No tiene otro uso en el server.

Pero, ya questamos, lo suyo sería como bien dices @rripio que se busque la vida el cliente (en el caso que no ocupa: la web)

@amiguelez , creo que el código implicado seria el javascrís del slider, creo que se podría hacer ahí el floor para que no haga un offset raruno en su rango operativo:

www/js/functions.js

function update_controls () {
...
...
    case 'level_page':

        if ($("#vol_slider").attr("value") != $php_data['level']) {
            $("#vol_slider").attr("value", $php_data['level']).slider("refresh");
        }
        if ($("#vol_slider").attr("max") != $php_data['maxlevel_i']) {
            $("#vol_slider").attr("max", $php_data['maxlevel_i']);
            $("#vol_slider").attr("min", $php_data['maxlevel_i'] - $config['vol_slider_hr']);
            $("#vol_slider").slider("refresh");
        }
rripio commented 7 years ago

Una pregunta: ¿falla actualmente la web sin el valor _maxleveli convertido a entero? No me ha quedado claro porque a @Rsantct le funcionaba antes de cambiar nada y a mí después de borrar cachés de navegadores...

rripio commented 7 years ago

Y otra pregunta. ¿Hay alguna razón, aparte de mostrar un número manejable, para que el intervalo mostrado en la web sea de 1 dB? ¿Es funcionamiento interno de los sliders?

Rsantct commented 7 years ago

Una pregunta: ¿falla actualmente la web sin el valor maxlevel_i convertido a entero? No me ha quedado claro porque a @Rsantct le funcionaba antes de cambiar nada y a mí después de borrar cachés de navegadores...

No no, la web va bien. Lo que pasa si se proporciona un maxlevel_i con decimales, es que "el rango operativo" del slider queda en límites con decimales, creo que es por la substracción que se hace en la penúltima línea de código que puese arriba. No debe ser complicado hacer adecuar ese rango haciendo floor ahí pero no se cómo :-/

En mi caso, solo he visto fallar el slider cuando se ha extraviado el archivo www/config/config.ini ya que no se puede leer vol_slider_hr, el scope del slider.

amiguelez commented 7 years ago

Para el tema del slider, propongo esta modificación que redondea a entero el valor de [maxlevel_i] :

function update_controls () {
...
...
    case 'level_page':

        if ($("#vol_slider").attr("value") != $php_data['level']) {
            $("#vol_slider").attr("value", $php_data['level']).slider("refresh");
        }
       if ($("#vol_slider").attr("max") != Math.round($php_data['maxlevel_i'])) {
            $("#vol_slider").attr("max",Math.round($php_data['maxlevel_i']));
            $("#vol_slider").attr("min",Math.round($php_data['maxlevel_i'])-$config['vol_slider_hr']);
            $("#vol_slider").slider("refresh");
        }

OjO está sin probar. Otra cosa, el paso que ahora es de 1dB es configurable con el atributo step. Si se ve interesante, es cuestión de poner una variable mas de configuración y que tanto el slider como los comandos de volume up y down utilicen este valor.

amiguelez commented 7 years ago

También se puede usar la función floor si se desea, en vez de round

Rsantct commented 7 years ago

Probado: ¡funciona bien! Ya lo he oficializado, y he quitado el redondeo que se enviaba desde server.

Por cierto, sobre el debate acerca de ese redondeo, al ser una facilidad meramente informativa de cara al exterior, imho me parece casi oportuno dar un valor redondeado.

@amiguelez : el paso de 1 dB en el slider me parece el adecuado, pasos más finos no los veo de utilidad y para más gruesos ya están los botones de 3 dBs.

Bien, llegados a este punto me estoy cuestionando si el slider pudiera ser prescindible ya que para mi gusto los botones cumplen las necesidades muy cómodamente. Es más: como se te vaya el dedo con el slider ¡la lías parda! :-P

amiguelez commented 7 years ago

Podemos poner una variable. Según su valor, habilitamos o no el slider. A gusto del usuario.

Enviado desde algún lugar

El 27 ago 2017, a las 12:58, Rsantct notifications@github.com escribió:

Probado: ¡funciona bien! Ya lo he oficializado, y he quitado el redondeo que se enviaba desde server.

Por cierto, sobre el debate acerca de ese redondeo, al ser una facilidad meramente informativa de cara al exterior, imho me parece casi oportuno dar un valor redondeado.

@amiguelez : el paso de 1 dB en el slider me parece el adecuado, pasos más finos no los veo de utilidad y para más gruesos ya están los botones de 3 dBs.

Bien, llegados a este punto me estoy cuestionando si el slider pudiera ser prescindible ya que para mi gusto los botones cumplen las necesidades muy cómodamente. Es más: como se te vaya el dedo con el slider ¡la lías parda! :-P

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Rsantct commented 7 years ago

Pues estaría muy bien, yo prometo usarla :-)

he cambiado la etiqueta a "enhancement"

rripio commented 7 years ago

El 27 de agosto de 2017, 12:58, Rsantct notifications@github.com escribió:

Probado: ¡funciona bien! Ya lo he oficializado, y he quitado el redondeo que se enviaba desde server.

Genial.

Por cierto, sobre el debate acerca de ese redondeo, al ser una facilidad meramente informativa de cara al exterior, imho me parece casi oportuno dar un valor redondeado.

No veo por qué hay que informar mal :-)

@amiguelez https://github.com/amiguelez : el paso de 1 dB en el slider me parece el adecuado, pasos más finos no los veo de utilidad y para más gruesos ya están los botones de 3 dBs.

No sé... Lo mejor ya que pensáis en quitarlo es dejarlo como está, pero a mí 0.5 dB me parece lo mínimo en high end...

-- Roberto Ripio

amiguelez commented 7 years ago

Yo pondría dos variables. Una para habilitar o no el slider. Otra para el paso que se desee. Este paso se puede usar tanto para el slider como para los botones de volume up y volume down. Incluso se pueden parametrizar los dos pasos, el que actualmente es 1dB y 3dB. Que os parece?

El 27 ago 2017, a las 19:43, Roberto Ripio notifications@github.com escribió:

El 27 de agosto de 2017, 12:58, Rsantct notifications@github.com escribió:

Probado: ¡funciona bien! Ya lo he oficializado, y he quitado el redondeo que se enviaba desde server.

Genial.

Por cierto, sobre el debate acerca de ese redondeo, al ser una facilidad meramente informativa de cara al exterior, imho me parece casi oportuno dar un valor redondeado.

No veo por qué hay que informar mal :-)

@amiguelez https://github.com/amiguelez : el paso de 1 dB en el slider me parece el adecuado, pasos más finos no los veo de utilidad y para más gruesos ya están los botones de 3 dBs.

No sé... Lo mejor ya que pensáis en quitarlo es dejarlo como está, pero a mí 0.5 dB me parece lo mínimo en high end...

-- Roberto Ripio — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

rripio commented 7 years ago

El 27 de agosto de 2017, 20:54, amiguelez notifications@github.com escribió:

Yo pondría dos variables. Una para habilitar o no el slider. Otra para el paso que se desee. Este paso se puede usar tanto para el slider como para los botones de volume up y volume down. Incluso se pueden parametrizar los dos pasos, el que actualmente es 1dB y 3dB. Que os parece?

Por mí bien.

-- Roberto Ripio

Rsantct commented 7 years ago

Hola olvide comentar esto. Por mi experiencia, los dos juegos de botones actuales +/- 1dB y +/- 3dB cumplen fenomenal, pero me parece genial que se pueda configurar como propones.

Rsantct commented 7 years ago

Plis decidme si os parece oportuno seguir adelante con las opciones de

Gracias!

Emmo, habiendo pasado un tiempo, dudo que valga la pena desarrollar estas opciones. Creo que es asumible que si usamos el slider, por su naturaleza va a redondear a 1 dB el volumen, y si no lo usamos el resto de botones y comandos ya dan un buen servicio.

amiguelez commented 7 years ago

Si tal lo podemos dejar de momento. Tampoco está mal así, otra cosa que por el manejo no veamos adecuado un slider (peligro de subir de golpe muchos dB) y podamos buscar otro tipo de control. O algún tipo de warning si se sube muchos dB.

Enviado desde algún lugar

El 19 sept 2017, a las 18:26, Rsantct notifications@github.com escribió:

Plis decidme si os parece oportuno seguir adelante con las opciones de

habilitar o no el slider de volumen configurar el paso del propio slider y/o los botones Gracias!

Emmo, habiendo pasado un tiempo, dudo que valga la pena desarrollar estas opciones. Creo que es asumible que si usamos el slider, por su naturaleza va a redondear a 1 dB el volumen, y si no lo usamos el resto de botones y comandos ya dan un buen servicio.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Rsantct commented 7 years ago

En mi expericencia, el peligro era el slider de balance que podía ser tocado sin querer ya que estaba en el límite inferior de la página en el móvil y casi no se nota el posible accidente. El de volumen está más centrado en la página es más difícil tocarlo sin querer y si que se nota su efecto ;-)

rripio commented 7 years ago

If ain't broken don't fix it.

El 19 de septiembre de 2017, 18:53, Rsantct notifications@github.com escribió:

En mi expericencia, el peligro era el slider de balance que podía ser tocado sin querer ya que estaba en el límite inferior de la página en el móvil y casi no se nota el posible accidente. El de volumen está más centrado en la página es más difícil tocarlo sin querer y si que se nota su efecto ;-)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/AudioHumLab/FIRtro/issues/7#issuecomment-330602853, or mute the thread https://github.com/notifications/unsubscribe-auth/AGkTERtpI1j2WX7bMhPGy0rQ11ElH49Fks5sj_FzgaJpZM4PCY6L .

-- Roberto Ripio

Rsantct commented 7 years ago

Bien pues, dejamos la issue cerrada entonces.