[vlc-devel] [PATCH 0/3] Bug when converting with both deinterlace checked and codec options

Jérôme Froissart software at froissart.eu
Thu Sep 19 22:46:18 CEST 2019


---
 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               |  81 +++++++++++
 modules/gui/qt/util/soutmrl.hpp               | 128 ++++++++++++++++++
 6 files changed, 213 insertions(+), 71 deletions(-)
 create mode 100644 modules/gui/qt/util/soutmrl.cpp
 create mode 100644 modules/gui/qt/util/soutmrl.hpp

diff --git a/modules/gui/qt/Makefile.am b/modules/gui/qt/Makefile.am
index af4a5d05cc..71f96a5910 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..92aa10ae90
--- /dev/null
+++ b/modules/gui/qt/util/soutmrl.cpp
@@ -0,0 +1,81 @@
+#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 )
+        {
+            s += "=" + it->second.to_string();
+        }
+        ++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..f584e1162b
--- /dev/null
+++ b/modules/gui/qt/util/soutmrl.hpp
@@ -0,0 +1,128 @@
+/*****************************************************************************
+ * soutmrl.hpp: A class to generate sout MRLs
+
****************************************************************************
+ *
+ *
+ *
+ *
+ * What should we put here?
+ * Authors: Jérôme Froissart <software at froissart.eu>
+ *
+ *
+ *
+ *
+ *
+
*****************************************************************************/
+
+#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=1,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


Le jeu. 19 sept. 2019 à 22:44, Jérôme Froissart <software at froissart.eu> a
écrit :

> Hi,
>
> When converting a video, an MRL is poorly handled.
> When the user checks the "deinterlace" statement, convert.cpp inserts this
> statement after removing closing braces to the MRL (using "mrl.remove( '}'
> );").
> This causes issues in case the passed mrl already contains closing braces
> (which is the case when the selected profile has custom video codec
> options).
> A quick-and-dirty fix would be to remove only the latest brace before
> inserting the "deinterlace" statement, but, well...that would be
> quick-and-dirty.
>
> I rather send you a more object-oriented implementation of SoutMrl, that
> avoids problems because it better handles "modules" and "options" within
> SoutMrls.
> The first and second patches are drop-in replacement for the current
> implementation.
> Then the thirs patch actually fixes the bug.
>
> To reproduce the bug:
> * open VLC, then "Open capture device" (I am not using the English
> version, my description can be slightly wrong)
> * Choose whatever source you want, e.g. screen
> * Choose "convert"
> * Choose a profile, and edit it. In the "video codec" tab, add a custom
> argument (e.g. "preset=faster", which makes sense with the x264 codec)
> * Start the process. VLC will fail because the MRL will lack a closing
> brace.
>   You can see the MRL in the "messages" dialog.
>   Without my patch, the MRL may look like
>
> sout=#transcode{vcodec=MJPG,venc=x264{preset=faster,scale=Auto,acodec=mp4a,ab=128,channels=1,samplerate=44100,scodec=none,deinterlace}:std{access=file{no-overwrite},mux=mp4,dst='/path/to/output.mp4'}
>   whereas my patch correctly handles the closing brace
>
>
> Sorry, I did not know what to put in the .hpp header, I could have
> copied/pasted the GPL license, but I do not know how to properly set the
> copyright. Maybe you'll know this better than I do.
>
> These patches can be applied (almost painlessly) to the 3.0.8 branch
> (there may only be slight issues with a Makefile.am and a convert.cpp that
> was slightly changed)
>
>
> Jérôme Froissart (3):
>   Re-implemented SoutMrl in a more object-oriented way
>   Additions to the new SoutMrl implementation
>   Deinterlace can be associated with codec arguments
>
>  modules/gui/qt/Makefile.am                    |   1 +
>  .../qt/components/sout/profile_selector.cpp   |  80 +++++-----
>  .../qt/components/sout/profile_selector.hpp   |   5 +-
>  .../gui/qt/components/sout/sout_widgets.cpp   |   1 +
>  modules/gui/qt/dialogs/convert.cpp            |  37 +++--
>  modules/gui/qt/dialogs/sout.cpp               |   4 +-
>  modules/gui/qt/dialogs/sout.hpp               |  71 +--------
>  modules/gui/qt/util/soutmrl.cpp               |  86 +++++++++++
>  modules/gui/qt/util/soutmrl.hpp               | 145 ++++++++++++++++++
>  9 files changed, 301 insertions(+), 129 deletions(-)
>  create mode 100644 modules/gui/qt/util/soutmrl.cpp
>  create mode 100644 modules/gui/qt/util/soutmrl.hpp
>
> --
> 2.20.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190919/a275a161/attachment.html>


More information about the vlc-devel mailing list