Merge lp:~e7appew/ubuntu/wily/bombono-dvd/mux-files-with-spaces-wily into lp:ubuntu/wily/bombono-dvd

[Carlos Maddela]

[Brian Murray (brian-murray)]

The patch's quoting of %2% is inconsistent, in one part it is bordered by \" and in another it isn't.

review: Needs Fixing

[Carlos Maddela (e7appew)]

It's been a while since I looked at this, but if I remember correctly,
when you quote %2% as well, all the rest of the arguments would be
treated as a single string, and the command fails.

On 08/06/16 04:51, Brian Murray wrote:
> Review: Needs Fixing
>
> The patch's quoting of %2% is inconsistent, in one part it is bordered by \" and in another it isn't.
>
> Diff comments:
>
>> === added file 'debian/patches/0002-Allow-muxing-of-files-with-spaces.patch'
>> --- debian/patches/0002-Allow-muxing-of-files-with-spaces.patch 1970-01-01 00:00:00 +0000
>> +++ debian/patches/0002-Allow-muxing-of-files-with-spaces.patch 2015-12-09 09:32:38 +0000
>> @@ -0,0 +1,39 @@
>> +From: Carlos Maddela <email address hidden>
>> +Date: Thu, 13 Aug 2015 15:05:04 +1000
>> +Subject: Allow muxing of files whose name or path contains spaces
>> +
>> +Description: Allow muxing of files whose name or path contains spaces.
>> + Arguments to mplex command should be quoted in case the files to be
>> + muxed have names or paths containing spaces.
>> +Author: Carlos Maddela <email address hidden>
>> +Origin: vendor
>> +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/bombono-dvd/+bug/1484397
>> +Last-Update: 2015-12-09
>> +---
>> +This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
>> +---
>> + src/mgui/mux.cpp | 4 ++--
>> + 1 file changed, 2 insertions(+), 2 deletions(-)
>> +
>> +diff --git a/src/mgui/mux.cpp b/src/mgui/mux.cpp
>> +index 427058e..cc4922b 100644
>> +--- a/src/mgui/mux.cpp
>> ++++ b/src/mgui/mux.cpp
>> +@@ -99,7 +99,7 @@ static bool RunMuxing(const std::string& dest_path, const std::string& args)
>> + ExitData ed;
>> + {
>> + Execution::Pulse pls(prg_bar);
>> +- std::string cmd = boost::format("mplex -f 8 -o %1% %2%") % dest_path % args % bf::stop;
>> ++ std::string cmd = boost::format("mplex -f 8 -o \"%1%\" %2%") % dest_path % args % bf::stop;
> Shouldn't %2% be treated the same way as %1% here?
>
>> + AppendCommandText(txt_view, cmd);
>> + ed = ExecuteAsync(0, cmd.c_str(), TextViewAppender(txt_view), &edat.pid);
>> + }
>> +@@ -234,7 +234,7 @@ bool MuxStreams(std::string& dest_fname, const std::string& src_fname)
>> + if( res )
>> + {
>> + dlg.hide();
>> +- res = RunMuxing(dest_fname, boost::format("%1% %2%") % v_btn.get_filename() % a_btn.get_filename() % bf::stop );
>> ++ res = RunMuxing(dest_fname, boost::format("\"%1%\" \"%2%\"") % v_btn.get_filename() % a_btn.get_filename() % bf::stop );
>> + }
>> +
>> + return res;
>

Unmerged revisions

27. By Carlos Maddela on 2015-12-09

Merge branch 'mux-files-with-spaces-vivid' into mux-files-with-spaces-wily

=== added directory '.pc/0002-Allow-muxing-of-files-with-spaces.patch'
=== added directory '.pc/0002-Allow-muxing-of-files-with-spaces.patch/src'
=== added directory '.pc/0002-Allow-muxing-of-files-with-spaces.patch/src/mgui'
=== added file '.pc/0002-Allow-muxing-of-files-with-spaces.patch/src/mgui/mux.cpp'
--- .pc/0002-Allow-muxing-of-files-with-spaces.patch/src/mgui/mux.cpp	1970-01-01 00:00:00 +0000
+++ .pc/0002-Allow-muxing-of-files-with-spaces.patch/src/mgui/mux.cpp	2015-12-09 09:32:38 +0000
@@ -0,0 +1,242 @@
+//
+// mgui/mux.cpp
+// This file is part of Bombono DVD project.
+//
+// Copyright (c) 2010 Ilya Murav'jov
+//
+// 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 St, Fifth Floor, Boston, MA  02110-1301  USA
+// 
+
+#include <mgui/_pc_.h>
+
+#include "mux.h"
+#include "execution.h" // ExecuteAsync()
+
+#include <mgui/sdk/textview.h>
+#include <mgui/sdk/packing.h>
+#include <mgui/sdk/widget.h>
+
+#include <mgui/execution.h>
+#include <mgui/gettext.h>
+#include <mgui/dialog.h>
+
+#include <mlib/filesystem.h>
+
+#include <gtk/gtkwindow.h> // gtk_window_set_geometry_hints()
+
+static void OnNewText(Gtk::TextView& txt_view, const char* dat, int sz, bool is_out,
+                      const ReadReadyFnr& add_fnr, const std::string& prefix)
+{
+    AppendNewText(txt_view, prefix + std::string(dat, sz), is_out);
+    if( add_fnr )
+        add_fnr(dat, sz, is_out);
+}
+
+ReadReadyFnr TextViewAppender(Gtk::TextView& txt_view, const ReadReadyFnr& add_fnr,
+                              const std::string& prefix)
+{
+    return bb::bind(&OnNewText, boost::ref(txt_view), _1, _2, _3, add_fnr, prefix);
+}
+
+static void OnResponse(Execution::Data& edat, int resp)
+{
+    if( resp == Gtk::RESPONSE_CANCEL)
+    {
+        // повтороне нажатие - закрытие диалога
+        if( !edat.userAbort )
+            edat.StopExecution(_("muxing"));
+    }
+}
+
+void SetDialogStrict(Gtk::Dialog& dlg, int min_wdh, int min_hgt, bool set_resizable)
+{
+    dlg.set_resizable(set_resizable); // чтоб при закрытии экспандера диалог уменьшался
+    // размер окна пошире
+    GdkGeometry geom;
+    geom.min_width  = min_wdh;
+    geom.min_height = min_hgt;
+    gtk_window_set_geometry_hints(static_cast<Gtk::Window&>(dlg).gobj(), 0, &geom, GDK_HINT_MIN_SIZE); 
+}
+
+static bool RunMuxing(const std::string& dest_path, const std::string& args)
+{
+    Gtk::Dialog dlg(BF_("Muxing \"%1%\"") % fs::name_str(dest_path) % bf::stop);
+    SetDialogStrict(dlg, 400, -1);
+
+    Gtk::TextView& txt_view = NewManaged<Gtk::TextView>();
+    Execution::Data edat;
+    Gtk::ProgressBar& prg_bar = NewManaged<Gtk::ProgressBar>();
+
+    {
+        Gtk::VBox& box = *dlg.get_vbox();
+        PackStart(box, prg_bar);
+
+        Gtk::Expander& expdr = PackStart(box, NewManaged<Gtk::Expander>(_("Show/_hide Details"), true));
+        txt_view.set_editable(false);
+        txt_view.set_size_request(0, 200);
+        expdr.add(PackDetails(txt_view));
+
+        dlg.get_action_area()->set_layout(Gtk::BUTTONBOX_CENTER);
+        dlg.add_button(Gtk::Stock::CANCEL, Gtk::RESPONSE_CANCEL);
+        dlg.signal_response().connect(bb::bind(&OnResponse, boost::ref(edat), _1));
+
+        dlg.show_all();
+    }
+
+    //dlg.run();
+    ExitData ed;
+    {
+        Execution::Pulse pls(prg_bar);
+        std::string cmd = boost::format("mplex -f 8 -o %1% %2%") % dest_path % args % bf::stop;
+        AppendCommandText(txt_view, cmd);
+        ed = ExecuteAsync(0, cmd.c_str(), TextViewAppender(txt_view), &edat.pid);
+    }
+
+    if( !ed.IsGood() && !edat.userAbort )
+    {
+        MessageBox(_("Muxing error"), Gtk::MESSAGE_ERROR, Gtk::BUTTONS_OK, 
+                   BF_("The reason is \"%1%\" (see Details)") % ExitDescription(ed) % bf::stop);
+        dlg.run();
+    }
+    return ed.IsGood();
+}
+
+struct SaveChooser: public Gtk::Table
+{
+                Gtk::Entry& ent;
+    Gtk::FileChooserButton& fcb;
+
+    SaveChooser(const char* type);
+};
+
+SaveChooser::SaveChooser(const char* type): 
+    ent(NewManaged<Gtk::Entry>()), 
+    fcb(NewManaged<Gtk::FileChooserButton>(_("Select a folder"), Gtk::FILE_CHOOSER_ACTION_SELECT_FOLDER))
+{
+    // вариант с виджетом - слишком большой, хоть и более функциональный
+    //Gtk::FileChooserWidget& fcw = NewManaged<Gtk::FileChooserWidget>(Gtk::FILE_CHOOSER_ACTION_SAVE);
+    //Gtk::Frame& frm = PackStart(vbox, NewManagedFrame(Gtk::SHADOW_ETCHED_IN, " Output: "), Gtk::PACK_EXPAND_WIDGET);
+    //Add(Add(frm, NewPaddingAlg(0, 5, 5, 5)), fcw);
+
+    set_col_spacings(5);
+    Gtk::Label& o_lbl = NewManaged<Gtk::Label>(type);
+    SetAlign(o_lbl, true);
+    attach(o_lbl, 0, 1, 0, 1, Gtk::SHRINK|Gtk::FILL);
+    attach(ent, 1, 2, 0, 1);
+    Gtk::Label& i_lbl = NewManaged<Gtk::Label>(_("in"));
+    SetAlign(i_lbl, false);
+    attach(i_lbl, 0, 1, 1, 2, Gtk::SHRINK|Gtk::FILL);
+    attach(fcb, 1, 2, 1, 2);
+}
+
+fs::path GetFilename(Gtk::FileChooser& fc)
+{
+    // fs::path v3 не понимает ustring
+    // однако в gtkmm >= будет возвращать std::string, и это не понадобится
+    return fs::path(fc.get_filename().raw());
+}
+
+std::string GetFilename(SaveChooser& sc)
+{
+    std::string fname = sc.ent.get_text();
+    if( !fname.empty() )
+        fname = (GetFilename(sc.fcb)/fname).string();
+    
+    return fname;
+}
+
+static void OnVideoSelected(Gtk::FileChooserButton& v_btn, Gtk::FileChooserButton& a_btn, SaveChooser& sc)
+{
+    fs::path pth = GetFilename(v_btn);
+    if( pth.empty() )
+        return;
+    std::string folder = pth.branch_path().string();
+
+    if( a_btn.get_filename().empty() )
+        a_btn.set_current_folder(folder);
+
+    if( GetFilename(sc).empty() )
+    {
+        sc.fcb.set_current_folder(folder);
+        sc.ent.set_text(get_basename(pth) + ".mpg");
+    }
+}
+
+bool MuxStreams(std::string& dest_fname, const std::string& src_fname)
+{
+    Gtk::Dialog dlg(_("Mux streams"));
+    SetDialogStrict(dlg, 400, -1);
+    SaveChooser& sc = NewManaged<SaveChooser>(SMCLN_("Output"));
+    Gtk::FileChooserButton& v_btn = NewManaged<Gtk::FileChooserButton>(_("Select elementary video"), Gtk::FILE_CHOOSER_ACTION_OPEN);
+    Gtk::FileChooserButton& a_btn = NewManaged<Gtk::FileChooserButton>(_("Select audio"),            Gtk::FILE_CHOOSER_ACTION_OPEN);
+    {
+        DialogVBox& vbox = AddHIGedVBox(dlg);
+
+        AppendWithLabel(vbox, v_btn, SMCLN_("Video"));
+        {
+            Gtk::FileFilter f;
+            f.set_name(_("MPEG2 elementary video (m2v)"));
+            f.add_pattern("*.m2v");
+            v_btn.add_filter(f);
+        }
+
+        AppendWithLabel(vbox, a_btn, SMCLN_("Audio"));
+        {
+            Gtk::FileFilter f;
+            f.set_name(_("Audio for DVD") + std::string(" (mp2/mpa, ac3, dts or 16bit lpcm)"));
+            FillSoundFilter(f);
+            a_btn.add_filter(f);
+        }
+
+        PackStart(vbox, sc);
+
+        CompleteDialog(dlg);
+    }
+
+    v_btn.signal_selection_changed().connect(bb::bind(&OnVideoSelected, boost::ref(v_btn), boost::ref(a_btn), boost::ref(sc)));
+    if( !src_fname.empty() )
+    {
+        if( get_extension(src_fname) == "m2v" )
+            v_btn.set_filename(src_fname);
+        else
+            a_btn.set_filename(src_fname);
+    }
+
+    bool res = false;
+    for( ; res = Gtk::RESPONSE_OK == dlg.run(), res; )
+    {
+        dest_fname = GetFilename(sc);
+
+        if( v_btn.get_filename().empty() )
+            ErrorBox(_("Elementary video file is not selected."));
+        else if( a_btn.get_filename().empty() )
+            ErrorBox(_("Audio file is not selected."));
+        else if( dest_fname.empty() )
+            ErrorBox(_("Output file name is empty."));
+        else if( CheckKeepOrigin(dest_fname) )
+            ;
+        else
+            break;
+    }
+
+    if( res )
+    {
+        dlg.hide();
+        res = RunMuxing(dest_fname, boost::format("%1% %2%") % v_btn.get_filename() % a_btn.get_filename() % bf::stop );
+    }
+
+    return res;
+}
+
 
=== modified file '.pc/applied-patches'
--- .pc/applied-patches	2015-08-12 12:16:58 +0000
+++ .pc/applied-patches	2015-12-09 09:32:38 +0000
@@ -1,2 +1,3 @@
 0001-ffmpeg-has-renamed-CodecID-AVCodecID.patch
+0002-Allow-muxing-of-files-with-spaces.patch
 1000-ubuntu-stop-using-ffmpeg-AVFormatContext-data_offset-now-private.patch
 
=== modified file 'debian/changelog'
--- debian/changelog	2015-09-07 08:27:53 +0000
+++ debian/changelog	2015-12-09 09:32:38 +0000
@@ -1,3 +1,10 @@
+bombono-dvd (1.2.2-0ubuntu9) wily; urgency=medium
+
+  * Allow muxing of files whose name or path contains spaces
+    (LP: #1484397).
+
+ -- Carlos Maddela <[e-mail]>  Wed, 09 Dec 2015 15:08:55 +1000
+
 bombono-dvd (1.2.2-0ubuntu8) wily; urgency=medium
 
   * No-change rebuild for libxml++ soname change.
 
=== added file 'debian/patches/0002-Allow-muxing-of-files-with-spaces.patch'
--- debian/patches/0002-Allow-muxing-of-files-with-spaces.patch	1970-01-01 00:00:00 +0000
+++ debian/patches/0002-Allow-muxing-of-files-with-spaces.patch	2015-12-09 09:32:38 +0000
@@ -0,0 +1,39 @@
+From: Carlos Maddela <[e-mail]>
+Date: Thu, 13 Aug 2015 15:05:04 +1000
+Subject: Allow muxing of files whose name or path contains spaces
+
+Description: Allow muxing of files whose name or path contains spaces.
+ Arguments to mplex command should be quoted in case the files to be
+ muxed have names or paths containing spaces.
+Author: Carlos Maddela <[e-mail]>
+Origin: vendor
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/bombono-dvd/+bug/1484397
+Last-Update: 2015-12-09
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+---
+ src/mgui/mux.cpp | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/src/mgui/mux.cpp b/src/mgui/mux.cpp
+index 427058e..cc4922b 100644
+--- a/src/mgui/mux.cpp
++++ b/src/mgui/mux.cpp
+@@ -99,7 +99,7 @@ static bool RunMuxing(const std::string& dest_path, const std::string& args)
+     ExitData ed;
+     {
+         Execution::Pulse pls(prg_bar);
+-        std::string cmd = boost::format("mplex -f 8 -o %1% %2%") % dest_path % args % bf::stop;
++        std::string cmd = boost::format("mplex -f 8 -o \"%1%\" %2%") % dest_path % args % bf::stop;
+         AppendCommandText(txt_view, cmd);
+         ed = ExecuteAsync(0, cmd.c_str(), TextViewAppender(txt_view), &edat.pid);
+     }
+@@ -234,7 +234,7 @@ bool MuxStreams(std::string& dest_fname, const std::string& src_fname)
+     if( res )
+     {
+         dlg.hide();
+-        res = RunMuxing(dest_fname, boost::format("%1% %2%") % v_btn.get_filename() % a_btn.get_filename() % bf::stop );
++        res = RunMuxing(dest_fname, boost::format("\"%1%\" \"%2%\"") % v_btn.get_filename() % a_btn.get_filename() % bf::stop );
+     }
+ 
+     return res;
 
=== modified file 'debian/patches/series'
--- debian/patches/series	2015-08-12 12:16:58 +0000
+++ debian/patches/series	2015-12-09 09:32:38 +0000
@@ -1,2 +1,3 @@
 0001-ffmpeg-has-renamed-CodecID-AVCodecID.patch
+0002-Allow-muxing-of-files-with-spaces.patch
 1000-ubuntu-stop-using-ffmpeg-AVFormatContext-data_offset-now-private.patch
 
=== modified file 'src/mgui/mux.cpp'
--- src/mgui/mux.cpp	2013-11-07 19:03:05 +0000
+++ src/mgui/mux.cpp	2015-12-09 09:32:38 +0000
@@ -99,7 +99,7 @@
     ExitData ed;
     {
         Execution::Pulse pls(prg_bar);
-        std::string cmd = boost::format("mplex -f 8 -o %1% %2%") % dest_path % args % bf::stop;
+        std::string cmd = boost::format("mplex -f 8 -o \"%1%\" %2%") % dest_path % args % bf::stop;
         AppendCommandText(txt_view, cmd);
         ed = ExecuteAsync(0, cmd.c_str(), TextViewAppender(txt_view), &edat.pid);
     }
@@ -234,7 +234,7 @@
     if( res )
     {
         dlg.hide();
-        res = RunMuxing(dest_fname, boost::format("%1% %2%") % v_btn.get_filename() % a_btn.get_filename() % bf::stop );
+        res = RunMuxing(dest_fname, boost::format("\"%1%\" \"%2%\"") % v_btn.get_filename() % a_btn.get_filename() % bf::stop );
     }
 
     return res;