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

[Yuval Tze]

Thanks for your review

About the tooltip and the suffix, the subsdelay filter has 3 calculation
modes which dictates different values

in short its
- absolute delay: new_delay = source_delay + factor
- (default) relative to source delay: new_delay = source_delay * factor
- relative to source content: new_delay = f(source_text) * factor

In the first mode the factor is actually seconds but in the other two it has
no units so I can't define a suffix
The same thing about the tip, it's hard to explain without considering the
mode

There are 2 options
- change the tip/suffix according to the mode
- assume that the default mode hasn't been changed

any preference/suggestions?


On Wed, May 11, 2011 at 2:11 AM, Jean-Baptiste Kempf <jb at videolan.org>wrote:

> 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20110511/a76e3715/attachment.html>