[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:54 CEST 2019
---
modules/gui/qt/util/soutmrl.cpp | 5 +++++
modules/gui/qt/util/soutmrl.hpp | 17 +++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/modules/gui/qt/util/soutmrl.cpp
b/modules/gui/qt/util/soutmrl.cpp
index 92aa10ae90..90615c6c86 100644
--- a/modules/gui/qt/util/soutmrl.cpp
+++ b/modules/gui/qt/util/soutmrl.cpp
@@ -38,6 +38,11 @@ void MrlModule::option( const QString& option )
options.append( OptionPairType( option, "" ) );
}
+QString SoutMrl::getHeader() const
+{
+ return hdr;
+}
+
QString SoutMrl::getMrl() const
{
QString mrl = hdr;
diff --git a/modules/gui/qt/util/soutmrl.hpp
b/modules/gui/qt/util/soutmrl.hpp
index f584e1162b..69ba4458bb 100644
--- a/modules/gui/qt/util/soutmrl.hpp
+++ b/modules/gui/qt/util/soutmrl.hpp
@@ -99,6 +99,17 @@ public:
{
}
+ void clear()
+ {
+ hdr = "";
+ modules.clear();
+ }
+
+ void header( const QString& newHeader )
+ {
+ hdr = newHeader;
+ }
+
MrlModule& begin( const QString& module )
{
modules.append( MrlModule( module ) );
@@ -110,6 +121,11 @@ public:
{
}
+ void module( const MrlModule& module )
+ {
+ modules.append( module );
+ }
+
// 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 );
@@ -117,6 +133,7 @@ public:
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 getHeader() const;
QString getMrl() const;
private:
--
2.20.1
Le jeu. 19 sept. 2019 à 22:46, Jérôme Froissart <software at froissart.eu> a
écrit :
>
> ---
> 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/dd4a5de3/attachment-0001.html>
More information about the vlc-devel
mailing list