[vlc-devel] [PATCH] Qt: synchronization: subtitles duration parameter

[Jean-Baptiste Kempf]

On Tue, May 10, 2011 at 03:05:01PM +0300, Yuval Tze wrote :
> - add a spin box to the synchronizations tab that enables the subsdelay filter when its value is changed and disables it when its value is set to 0.

This lacks setToolTips and other niceties to understand what it actually
do...

> - make the private function ExtVideo::ChangeVFiltersString accessible to other local panels.
OK-ish...

> -        p_vout = THEMIM->getVout();
> +        vout_thread_t *p_vout = THEMIM->getVout();

Then you should remove p_vout from the class member (and fix the small
issue that will ensue...)

> +    subDurationSpin = new QDoubleSpinBox;
> +    subDurationSpin->setAlignment( Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter );
> +    subDurationSpin->setDecimals( 3 );
> +    subDurationSpin->setMinimum( 0 );
> +    subDurationSpin->setMaximum( 20 );
> +    subDurationSpin->setSingleStep( 0.2 );
> +    subsLayout->addWidget( subDurationSpin, 2, 2, 1, 1 );
No suffix?

> +    vlc_value_t val;
> +
> +    val.f_float = f_factor;
> +    if( var_SetChecked( p_intf->p_libvlc, CFG_SUBSDELAY_FACTOR, VLC_VAR_FLOAT, val ) )
> +    {
> +        var_Create( p_intf->p_libvlc, CFG_SUBSDELAY_FACTOR, VLC_VAR_FLOAT | VLC_VAR_DOINHERIT );
> +        var_SetFloat( p_intf->p_libvlc, CFG_SUBSDELAY_FACTOR, f_factor );
> +    }
Are you sure that you need all that?
If so, you need to document it...

> +double SyncControls::subsdelayGetFactor()
> +{
> +    vlc_value_t val;
> +
> +    if( !var_GetChecked( p_intf->p_libvlc, CFG_SUBSDELAY_FACTOR, VLC_VAR_FLOAT, &val ) )
> +    {
> +        return val.f_float;
> +    }
I am confused here the same...

Best Regards,

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device