[vlc-devel] VLC MiniMode Widget: SVG/CSS based Media Playback Notification and Controller

jpd at videolan.org jpd at videolan.org
Fri Jun 5 01:48:16 CEST 2009


On Fri, Jun 05, 2009 at 12:40:37AM +0200, Rohit Yadav wrote:
> +/* Volume Control Interfaces */
> +#define MNW_MAX_VOLUME 200
> +void MiniModeWidget::notifierVolumeChanged( int i_vol )
> +{
> +    int i_res = ( i_vol )  * ( AOUT_VOLUME_MAX / 2 ) / MNW_MAX_VOLUME;
> +    aout_VolumeSet( p_intf, i_res );
> +}

That probably shoulnd't be an int, judging by the following.


> +void MiniModeWidget::updateNotifierVolume( void )
> +{
> +    audio_volume_t i_volume;
> +    aout_VolumeGet( p_intf, &i_volume );
> +    i_volume = ( ( i_volume + 1 ) *   MNW_MAX_VOLUME )/ ( AOUT_VOLUME_MAX / 2 );
> +
> +    volumeControl->setValue( i_volume );
> +}

Provided the largest value still fits in the integer type used, it is
usually a better idea to do multiplications before divisions, and if you
can to minimize the total number of operations actually executed. For
example, by re-ordering the calculations and packing as much constants
together so that the compiler can do them instead of emiting code to do
them at run time. Here, you're using parentheses to force the opposite.


> +/* Sync UI Elements with Main Interface UI */
> +void MiniModeWidget::syncNotifierController( const int &i_status )
> +{
> +    switch( i_status )
> +    {
> +        case 2: svgPlay->load(tr(":/svgs/pause.svg") ); b_toggle = false; break;
> +        case 4: /* STOP */ 
> +        case 3: svgPlay->load(tr(":/svgs/play.svg") );  b_toggle = true;  break;
> +        default: break; 
> +    }
> +}

States are generally better expressed with enumerated types instead of
bare numbers. Documenting the fall-through would be good, too.


I don't know enough about Qt to know whether the following applies:
If methods are a) small enough (like, two or three lines not counting
comments or curly braces) and b) not virtual, then making them inlinable
can potentially lead to substantial savings in reduced call overhead.




More information about the vlc-devel mailing list