[vlc-devel] [PATCH v2 1/3] Re-implemented SoutMrl in a more object-oriented way
Thomas Guillem
thomas at gllm.fr
Thu Oct 17 17:18:58 CEST 2019
Hello,
I'm sorry in advance, I have only comments about some small details (nitpicking)
On Thu, Oct 17, 2019, at 17:11, Jérôme Froissart wrote:
> ---
> AUTHORS | 1 +
> modules/gui/qt/Makefile.am | 1 +
> .../qt/components/sout/profile_selector.cpp | 2 +-
> .../gui/qt/components/sout/sout_widgets.cpp | 1 +
> modules/gui/qt/dialogs/sout.hpp | 71 +---------
> modules/gui/qt/util/soutmrl.cpp | 108 ++++++++++++++
> modules/gui/qt/util/soutmrl.hpp | 133 ++++++++++++++++++
> 7 files changed, 246 insertions(+), 71 deletions(-)
> create mode 100644 modules/gui/qt/util/soutmrl.cpp
> create mode 100644 modules/gui/qt/util/soutmrl.hpp
>
> diff --git a/AUTHORS b/AUTHORS
> index 3b6a86c9f9..402f5e2dfb 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -702,6 +702,7 @@ Jed Smith
> Jérémy Carrier
> Jeremy Huddleston Sequoia
> Jerome Forissier
> +Jérôme Froissart
You should not touch the AUTHORS, we have a script that update it (we generally run it before a release).
> Jim Bankoski
> Jonas Lundqvist
> Jonatan "jaw" Wallmander
> diff --git a/modules/gui/qt/Makefile.am b/modules/gui/qt/Makefile.am
> index 2605d17b0f..9867600cd7 100644
> --- a/modules/gui/qt/Makefile.am
> +++ b/modules/gui/qt/Makefile.am
> @@ -186,6 +186,7 @@ libqt_plugin_la_SOURCES = \
> gui/qt/util/customwidgets.cpp gui/qt/util/customwidgets.hpp \
> gui/qt/util/searchlineedit.cpp gui/qt/util/searchlineedit.hpp \
> gui/qt/util/registry.cpp gui/qt/util/registry.hpp \
> + gui/qt/util/soutmrl.cpp gui/qt/util/soutmrl.hpp \
> gui/qt/util/qt_dirs.cpp gui/qt/util/qt_dirs.hpp \
> gui/qt/util/validators.cpp gui/qt/util/validators.hpp \
> gui/qt/util/buttons/BrowseButton.cpp \
> diff --git a/modules/gui/qt/components/sout/profile_selector.cpp
> b/modules/gui/qt/components/sout/profile_selector.cpp
> index 586a01f681..55fc5c9c28 100644
> --- a/modules/gui/qt/components/sout/profile_selector.cpp
> +++ b/modules/gui/qt/components/sout/profile_selector.cpp
> @@ -23,7 +23,7 @@
>
> #include "components/sout/profile_selector.hpp"
> #include "components/sout/profiles.hpp"
> -#include "dialogs/sout.hpp"
> +#include "util/soutmrl.hpp"
>
> #include <QHBoxLayout>
> #include <QToolButton>
> diff --git a/modules/gui/qt/components/sout/sout_widgets.cpp
> b/modules/gui/qt/components/sout/sout_widgets.cpp
> index 8eb9cccea3..88e64f42cc 100644
> --- a/modules/gui/qt/components/sout/sout_widgets.cpp
> +++ b/modules/gui/qt/components/sout/sout_widgets.cpp
> @@ -26,6 +26,7 @@
> #include "components/sout/sout_widgets.hpp"
> #include "dialogs/sout.hpp"
> #include "util/qt_dirs.hpp"
> +#include "util/soutmrl.hpp"
> #include <vlc_intf_strings.h>
>
> #include <QGroupBox>
> diff --git a/modules/gui/qt/dialogs/sout.hpp b/modules/gui/qt/dialogs/sout.hpp
> index 909dcf80bf..9380a0bc5b 100644
> --- a/modules/gui/qt/dialogs/sout.hpp
> +++ b/modules/gui/qt/dialogs/sout.hpp
> @@ -31,81 +31,12 @@
>
> #include "ui_sout.h"
> #include "util/qvlcframe.hpp"
> +#include "util/soutmrl.hpp"
>
> #include <QWizard>
>
> class QPushButton;
>
> -class SoutMrl
> -{
> -public:
> - SoutMrl( const QString& head = "")
> - {
> - mrl = head;
> - b_first = true;
> - b_has_bracket = false;
> - }
> -
> - QString getMrl()
> - {
> - return mrl;
> - }
> -
> - void begin( const QString& module )
> - {
> - if( !b_first )
> - mrl += ":";
> - b_first = false;
> -
> - mrl += module;
> - b_has_bracket = false;
> - }
> - void end()
> - {
> - if( b_has_bracket )
> - mrl += "}";
> - }
> - void option( const QString& option, const QString& value = "" )
> - {
> - if( !b_has_bracket )
> - mrl += "{";
> - else
> - mrl += ",";
> - b_has_bracket = true;
> -
> - mrl += option;
> -
> - if( !value.isEmpty() )
> - {
> - char *psz = config_StringEscape( qtu(value) );
> - if( psz )
> - {
> - mrl += "=" + qfu( psz );
> - free( psz );
> - }
> - }
> - }
> - void option( const QString& name, const int i_value, const int
> i_precision = 10 )
> - {
> - option( name, QString::number( i_value, i_precision ) );
> - }
> - void option( const QString& name, const double f_value )
> - {
> - option( name, QString::number( f_value ) );
> - }
> -
> - void option( const QString& name, const QString& base, const int
> i_value, const int i_precision = 10 )
> - {
> - option( name, base + ":" + QString::number( i_value,
> i_precision ) );
> - }
> -
> -private:
> - QString mrl;
> - bool b_has_bracket;
> - bool b_first;
> -};
> -
> -
> class SoutDialog : public QWizard
> {
> Q_OBJECT
> diff --git a/modules/gui/qt/util/soutmrl.cpp
> b/modules/gui/qt/util/soutmrl.cpp
> new file mode 100644
> index 0000000000..5452f8cccb
> --- /dev/null
> +++ b/modules/gui/qt/util/soutmrl.cpp
> @@ -0,0 +1,108 @@
> +/*****************************************************************************
> + * soutmrl.cpp: A class to generate sout MRLs
> +
> ****************************************************************************
> + * Copyright (C) 2019 Jérôme Froissart <software at froiss.art>
> + *
> + * Authors: Jérôme Froissart <software at froiss.art>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA
> 02110-1301, USA.
> +
> *****************************************************************************/
> +
> +#include "soutmrl.hpp"
> +
> +QString MrlModule::to_string() const
> +{
> + QString s = moduleName;
> +
> + if( options.size() > 0 )
> + {
> + s += "{";
> + }
> + OptionsType::const_iterator it;
> + for( it=options.begin(); it!=options.end(); )
> + {
> + s += it->first;
> + if( it->second.to_string().compare("") != 0 )
> + {
> + char *psz = config_StringEscape(
> qtu(it->second.to_string()) );
> + if( psz )
> + {
> + s += "=" + qfu( psz );
> + free( psz );
> + }
> + }
> + ++it;
> + if( it != options.end() )
> + {
> + s += ",";
> + }
> + }
> + if( options.size() > 0 )
> + {
> + s += "}";
> + }
> + return s;
> +}
> +
> +void MrlModule::option( const QString& option, const MrlOption& value )
> +{
> + options.append( OptionPairType( option, value ) );
> +}
> +void MrlModule::option( const QString& option )
> +{
> + options.append( OptionPairType( option, "" ) );
> +}
> +
> +QString SoutMrl::getMrl() const
> +{
> + QString mrl = hdr;
> + for( int m=0; m<modules.size(); m++ )
> + {
> + mrl += modules[m].to_string();
> + if( m < modules.size() - 1 )
> + {
> + mrl += ":";
> + }
> + }
> +
> + return mrl;
> +}
> +
> +void SoutMrl::option( const QString& name, const QString& value )
> +{
> + if( modules.size() > 0 )
> + {
> + modules.back().option( name, value );
> + }
> +}
> +void SoutMrl::option( const QString& name, const int i_value, const
> int i_precision )
> +{
> + option( name, QString::number( i_value, i_precision ) );
> +}
> +void SoutMrl::option( const QString& name, const double f_value )
> +{
> + option( name, QString::number( f_value ) );
> +}
> +void SoutMrl::option( const QString& name, const QString& base, const
> int i_value, const int i_precision )
> +{
> + option( name, base + ":" + QString::number( i_value, i_precision )
> );
> +}
> +void SoutMrl::option( const QString& name, const MrlModule& nested )
> +{
> + if( modules.size() > 0 )
> + {
> + modules.back().option( name, nested );
> + }
> +}
> \ No newline at end of file
> diff --git a/modules/gui/qt/util/soutmrl.hpp
> b/modules/gui/qt/util/soutmrl.hpp
> new file mode 100644
> index 0000000000..00097ce82a
> --- /dev/null
> +++ b/modules/gui/qt/util/soutmrl.hpp
> @@ -0,0 +1,133 @@
> +/*****************************************************************************
> + * soutmrl.hpp: A class to generate sout MRLs
> +
> ****************************************************************************
> + * Copyright (C) 2019 Jérôme Froissart <software at froiss.art>
> + *
> + * Authors: Jérôme Froissart <software at froiss.art>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA
> 02110-1301, USA.
> +
> *****************************************************************************/
> +
> +#ifndef VLC_QT_SOUTMRL_HPP_
> +#define VLC_QT_SOUTMRL_HPP_
> +
> +#include "qt.hpp"
> +
> +#include <QMap>
> +
> +
> +class MrlOption;
> +
> +class MrlModule
> +{
> +public:
> + MrlModule( const QString& name ) :
> + moduleName( name )
> + {
> + }
> +
> + void option( const QString& option, const MrlOption& value );
> + void option( const QString& option );
> + QString to_string() const;
> +
> +private:
> + typedef QPair<QString, MrlOption> OptionPairType;
> + typedef QList<OptionPairType> OptionsType;
> + const QString moduleName;
> + OptionsType options;
> +};
> +
> +
> +class MrlOption
> +{
> +public:
> + MrlOption( const QString& value ) :
> + kind( String ),
> + stringValue( value ),
> + nestedModule("")
> + {}
> +
> + MrlOption( const char* s ) :
> + MrlOption( QString(s) )
> + {}
> +
> + MrlOption( const MrlModule& module ) :
> + kind(Nested),
> + nestedModule(module)
> + {}
> +
> + QString to_string() const{
> + if( kind == String )
> + {
> + return stringValue;
> + }
> + else
> + {
> + return nestedModule.to_string();
> + }
> + }
> +
> +private:
> + enum Kind{ String, Nested };
> + const Kind kind;
> + const QString stringValue;
> + const MrlModule nestedModule;
> +};
> +
> +
> +/// This class helps building MRLs
> +///
> +/// An MRL has the following structure:
> +/// * a header
> +/// * any number of modules, which have
> +/// - a name
> +/// - any number of key(=value) pairs
> +/// values can be nested modules
> +///
> +/// Example of MRL:
> HEADERmodule1{a,b=val}:module2:module3{opt,arg=\"value with
> automatically escaped quotes\",stuff=nestedModule{subkey=subvalue}}
> +class SoutMrl
> +{
> +public:
> + SoutMrl( const QString& header="" ) :
> + hdr(header)
> + {
> + }
> +
> + MrlModule& begin( const QString& module )
> + {
> + modules.append( MrlModule( module ) );
> + return modules.back();
> + }
> +
> + // Useless, kept for compatibility with an older API
> + void end()
> + {
> + }
> +
> + // These should be only in MRLModule, but they are kept in this
> parent class for compatibility with an older API
> + void option( const QString& name, const QString& value = "" );
> + void option( const QString& name, const int i_value, const int
> i_precision = 10 );
> + void option( const QString& name, const double f_value );
> + void option( const QString& name, const QString& base, const int
> i_value, const int i_precision = 10 );
> + void option( const QString& name, const MrlModule& nested );
> +
> + QString getMrl() const;
> +
> +private:
> + QString hdr;
> + QList<MrlModule> modules;
> +};
> +
> +#endif // include guard
> --
> 2.20.1
>
And a small remark about the commit title:
You should prefix it with "qt:". That way we know that it is a qt commit.
You should also use the imperative mood, cf. https://git.kernel.org/pub/scm/git/git.git/tree/Documentation/SubmittingPatches?id=HEAD#n133
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list