<div dir="ltr"><div>Hi,</div><div><br></div>It has to be ::Clip because Clip would correspond to Commands::Clip in that scope and that would result in a compilation error.<div>The temporary variable was needed because in the following connect if i had written simply &Transcoder::progress, then compiler would find it ambiguous because Transcoder::progress</div><div>can have either int or float as a parameter.</div><div>Should i submit a patch which changes <span style="color:rgb(128,128,0)">void</span><span style="color:rgb(192,192,192)"> </span><span style="color:rgb(0,0,0)">(</span><span style="color:rgb(0,0,0)">NotificationZone</span><span style="color:rgb(0,0,0)">::*</span><span style="color:rgb(0,0,0)">f</span><span style="color:rgb(0,0,0)">)(</span><span style="color:rgb(128,128,0)">float</span><span style="color:rgb(0,0,0)">) to auto?</span></div>
</div><div class="gmail_extra"><br><div class="gmail_quote">2016-03-14 21:13 GMT+01:00 Hugo Beauzée-Luyssen <span dir="ltr"><<a href="mailto:hugo@beauzee.fr" target="_blank">hugo@beauzee.fr</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<div><div class="h5"><br>
On 03/12/2016 01:30 PM, Paweł Wegner wrote:<br>
> ---<br>
> src/Commands/Commands.cpp | 11 +++++++----<br>
> src/Commands/KeyboardShortcutHelper.cpp | 4 ++--<br>
> src/Library/Library.cpp | 3 +++<br>
> src/Main/Core.cpp | 6 +++---<br>
> src/Main/main.cpp | 1 +<br>
> src/Media/Clip.cpp | 4 ++--<br>
> src/Media/Transcoder.cpp | 13 +++++++------<br>
> 7 files changed, 25 insertions(+), 17 deletions(-)<br>
><br>
> diff --git a/src/Commands/Commands.cpp b/src/Commands/Commands.cpp<br>
> index 9351107..bfa291f 100644<br>
> --- a/src/Commands/Commands.cpp<br>
> +++ b/src/Commands/Commands.cpp<br>
> @@ -43,6 +43,9 @@ Commands::Generic::Generic() :<br>
> {<br>
> //This is connected using a direct connection to ensure the view can be refreshed<br>
> //just after the signal has been emited.<br>
> +<br>
> + //FIXME: there is no signal retranslateRequired in QUndoStack class<br>
> + // <3 qt4 connects<br>
> connect( Core::getInstance()->undoStack(), SIGNAL( retranslateRequired() ),<br>
> this, SLOT( retranslate() ), Qt::DirectConnection );<br>
> }<br>
> @@ -80,7 +83,7 @@ Commands::Clip::Add::Add( ClipHelper* ch, TrackWorkflow* tw, qint64 pos ) :<br>
> m_trackWorkflow( tw ),<br>
> m_pos( pos )<br>
> {<br>
> - connect( ch->clip(), SIGNAL( destroyed() ), this, SLOT( invalidate() ) );<br>
> + connect( ch->clip(), &::Clip::destroyed, this, &Add::invalidate );<br>
<br>
</div></div>Why do you prepend by a scope resolution operator? Why not simply<br>
Clip::destroyed?<br>
Same question goes for below<br>
<div><div class="h5"><br>
> retranslate();<br>
> }<br>
><br>
> @@ -158,7 +161,7 @@ Commands::Clip::Move::internalUndo()<br>
> Commands::Clip::Remove::Remove( ClipHelper* ch, TrackWorkflow* tw ) :<br>
> m_clipHelper( ch ), m_trackWorkflow( tw )<br>
> {<br>
> - connect( ch->clip(), SIGNAL( destroyed() ), this, SLOT( invalidate() ) );<br>
> + connect( ch->clip(), &::Clip::destroyed, this, &Remove::invalidate );<br>
> retranslate();<br>
> m_pos = tw->getClipPosition( ch->uuid() );<br>
> }<br>
> @@ -189,7 +192,7 @@ Commands::Clip::Resize::Resize( TrackWorkflow* tw, ClipHelper* ch, qint64 newBeg<br>
> m_newEnd( newEnd ),<br>
> m_newPos( newPos )<br>
> {<br>
> - connect( ch->clip(), SIGNAL( destroyed() ), this, SLOT( invalidate() ) );<br>
> + connect( ch->clip(), &::Clip::destroyed, this, &Resize::invalidate );<br>
> m_oldBegin = ch->begin();<br>
> m_oldEnd = ch->end();<br>
> m_oldPos = tw->getClipPosition( ch->uuid() );<br>
> @@ -230,7 +233,7 @@ Commands::Clip::Split::Split( TrackWorkflow *tw, ClipHelper *toSplit,<br>
> m_newClipPos( newClipPos ),<br>
> m_newClipBegin( newClipBegin )<br>
> {<br>
> - connect( toSplit->clip(), SIGNAL( destroyed() ), this, SLOT( invalidate() ) );<br>
> + connect( toSplit->clip(), &::Clip::destroyed, this, &Split::invalidate );<br>
> m_newClip = new ClipHelper( toSplit->clip(), newClipBegin, toSplit->end() );<br>
> m_oldEnd = toSplit->end();<br>
> retranslate();<br>
> diff --git a/src/Commands/KeyboardShortcutHelper.cpp b/src/Commands/KeyboardShortcutHelper.cpp<br>
> index 58a834c..eb76bb4 100644<br>
> --- a/src/Commands/KeyboardShortcutHelper.cpp<br>
> +++ b/src/Commands/KeyboardShortcutHelper.cpp<br>
> @@ -33,7 +33,7 @@ KeyboardShortcutHelper::KeyboardShortcutHelper( const QString& name, QWidget* pa<br>
> {<br>
> SettingValue* setting = Core::getInstance()->settings()->value( name );<br>
> setKey( QKeySequence( setting->get().toString() ) );<br>
> - connect( setting, SIGNAL( changed( QVariant ) ), this, SLOT( shortcutUpdated( const QVariant& ) ) );<br>
> + connect( setting, &SettingValue::changed, this, &KeyboardShortcutHelper::shortcutUpdated );<br>
> }<br>
><br>
> KeyboardShortcutHelper::KeyboardShortcutHelper( const QString& name, QAction *action,<br>
> @@ -44,7 +44,7 @@ KeyboardShortcutHelper::KeyboardShortcutHelper( const QString& name, QAction *ac<br>
> {<br>
> SettingValue* setting = Core::getInstance()->settings()->value( name );<br>
> action->setShortcut( setting->get().toString() );<br>
> - connect( setting, SIGNAL( changed( QVariant ) ), this, SLOT( shortcutUpdated( const QVariant& ) ) );<br>
> + connect( setting, &SettingValue::changed, this, &KeyboardShortcutHelper::shortcutUpdated );<br>
> }<br>
><br>
> void<br>
> diff --git a/src/Library/Library.cpp b/src/Library/Library.cpp<br>
> index 5e102a3..9685280 100644<br>
> --- a/src/Library/Library.cpp<br>
> +++ b/src/Library/Library.cpp<br>
> @@ -114,6 +114,9 @@ Library::mediaLoaded( const Media* media )<br>
> {<br>
> if ( media != NULL )<br>
> {<br>
> + //FIXME: metaDataComputed in Media class has no arguments, this shouldn't be working;<br>
> + //replacing this with disconnect( media, &Media::metaDataComputed, this, &Library::mediaLoaded );<br>
> + //produces static_assert error<br>
> disconnect( media, SIGNAL( metaDataComputed( const Media* ) ),<br>
> this, SLOT( mediaLoaded( const Media* ) ) );<br>
> }<br>
> diff --git a/src/Main/Core.cpp b/src/Main/Core.cpp<br>
> index a9fcf0b..7145478 100644<br>
> --- a/src/Main/Core.cpp<br>
> +++ b/src/Main/Core.cpp<br>
> @@ -55,9 +55,9 @@ Core::Core()<br>
> m_library = new Library( m_workspace );<br>
> m_currentProject = new Project( m_settings );<br>
><br>
> - connect( m_undoStack, SIGNAL( cleanChanged( bool ) ), m_currentProject, SLOT( cleanChanged( bool ) ) );<br>
> - connect( m_currentProject, SIGNAL( projectSaved() ), m_undoStack, SLOT( setClean() ) );<br>
> - connect( m_library, SIGNAL( cleanStateChanged( bool ) ), m_currentProject, SLOT( libraryCleanChanged( bool ) ) );<br>
> + connect( m_undoStack, &QUndoStack::cleanChanged, m_currentProject, &Project::cleanChanged );<br>
> + connect( m_currentProject, &Project::projectSaved, m_undoStack, &QUndoStack::setClean );<br>
> + connect( m_library, &Library::cleanStateChanged, m_currentProject, &Project::libraryCleanChanged );<br>
> connect( m_currentProject, SIGNAL( projectLoaded( QString, QString ) ),<br>
> m_recentProjects, SLOT( projectLoaded( QString, QString ) ) );<br>
> }<br>
> diff --git a/src/Main/main.cpp b/src/Main/main.cpp<br>
> index f2e9b49..6003d7b 100644<br>
> --- a/src/Main/main.cpp<br>
> +++ b/src/Main/main.cpp<br>
> @@ -193,6 +193,7 @@ VLMCCoremain( int argc, char **argv )<br>
> ConsoleRenderer renderer;<br>
> ProjectManager *pm = Core::getInstance()->currentProject();<br>
><br>
> + //FIXME: only signal projectLoaded( const QString& ) in ProjectManager<br>
> QCoreApplication::connect( pm, SIGNAL( projectLoaded() ), &renderer, SLOT( startRender() ) );<br>
> pm->loadProject( app.arguments()[1] );<br>
> #endif<br>
> diff --git a/src/Media/Clip.cpp b/src/Media/Clip.cpp<br>
> index 2c4a491..94535b3 100644<br>
> --- a/src/Media/Clip.cpp<br>
> +++ b/src/Media/Clip.cpp<br>
> @@ -47,8 +47,8 @@ Clip::Clip( Media *media, qint64 begin /*= 0*/, qint64 end /*= -1*/, const QStri<br>
> m_childs = new MediaContainer( this );<br>
> m_rootClip = media->baseClip();<br>
> computeLength();<br>
> - connect( media, SIGNAL( metaDataComputed() ),<br>
> - this, SLOT( mediaMetadataUpdated() ) );<br>
> + connect( media, &Media::metaDataComputed,<br>
> + this, &Clip::mediaMetadataUpdated );<br>
> }<br>
><br>
> Clip::Clip( Clip *parent, qint64 begin /*= -1*/, qint64 end /*= -1*/,<br>
> diff --git a/src/Media/Transcoder.cpp b/src/Media/Transcoder.cpp<br>
> index 1357329..7640d31 100644<br>
> --- a/src/Media/Transcoder.cpp<br>
> +++ b/src/Media/Transcoder.cpp<br>
> @@ -36,10 +36,11 @@ Transcoder::Transcoder( Media* media )<br>
> : m_media( media )<br>
> , m_renderer( NULL )<br>
> {<br>
> - connect( this, SIGNAL( notify( QString ) ),<br>
> - NotificationZone::getInstance(), SLOT( notify( QString ) ) );<br>
> - connect( this, SIGNAL( progress( float ) ),<br>
> - NotificationZone::getInstance(), SLOT( progressUpdated( float ) ) );<br>
> + connect( this, &Transcoder::notify,<br>
> + NotificationZone::getInstance(), &NotificationZone::notify );<br>
> + void (NotificationZone::*f)(float) = &NotificationZone::progressUpdated;<br>
<br>
</div></div>Why the temporary variable? Also, feel free to use "auto" ;)<br>
<span class=""><br>
> + connect( this, &Transcoder::progress,<br>
> + NotificationZone::getInstance(), f );<br>
> m_eventWatcher = new RendererEventWatcher;<br>
> }<br>
><br>
> @@ -62,8 +63,8 @@ Transcoder::transcodeToPs()<br>
> m_destinationFile = outputDir + '/' + m_media->fileInfo()->baseName() + ".ps";<br>
> m_renderer->setOutputFile( qPrintable( m_destinationFile ) );<br>
> m_renderer->setName( qPrintable( QString( "Transcoder " ) + m_media->fileInfo()->baseName() ) );<br>
> - connect( m_eventWatcher, SIGNAL( positionChanged( float ) ), this, SIGNAL( progress( float ) ) );<br>
> - connect( m_eventWatcher, SIGNAL( endReached() ), this, SLOT( transcodeFinished() ) );<br>
> + connect( m_eventWatcher, &RendererEventWatcher::positionChanged, this, &Transcoder::progress );<br>
> + connect( m_eventWatcher, &RendererEventWatcher::endReached, this, &Transcoder::transcodeFinished );<br>
> emit notify( "Transcoding " + m_media->fileInfo()->absoluteFilePath() + " to " + m_destinationFile );<br>
> m_renderer->start();<br>
> }<br>
><br>
<br>
</span>Except for those remarks, looks good to me!<br>
<br>
Regards,<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Hugo Beauzée-Luyssen<br>
<a href="http://www.beauzee.fr" rel="noreferrer" target="_blank">www.beauzee.fr</a><br>
_______________________________________________<br>
Vlmc-devel mailing list<br>
<a href="mailto:Vlmc-devel@videolan.org">Vlmc-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/vlmc-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlmc-devel</a><br>
</font></span></blockquote></div><br></div>