justdan96 / tsMuxer

tsMuxer is a transport stream muxer for remuxing/muxing elementary streams, EVO/VOB/MPG, MKV/MKA, MP4/MOV, TS, M2TS to TS to M2TS. Supported video codecs H.264/AVC, H.265/HEVC, VC-1, MPEG2. Supported audio codecs AAC, AC3 / E-AC3(DD+), DTS/ DTS-HD.
Apache License 2.0
853 stars 144 forks source link

can <filesystem> usage be optional? #577

Closed Randrianasulu closed 2 years ago

Randrianasulu commented 2 years ago

Tried to compile tsmuxer git on my portable Slackware system (custom), neraly everything was working with clang 10 installed separately, but at very end usage of this (c++17?) feature errored out.

I killed usage with hack like this:

diff --git a/tsMuxer/osdep/textSubtitlesRenderFT.cpp b/tsMuxer/osdep/textSubtitlesRenderFT.cpp
index 198fceb..420dafa 100644
--- a/tsMuxer/osdep/textSubtitlesRenderFT.cpp
+++ b/tsMuxer/osdep/textSubtitlesRenderFT.cpp
@@ -27,7 +27,7 @@ const static char FONT_ROOT[] = "/System/Library/Fonts/";
 #include <freetype/ftstroke.h>
 #include <fs/systemlog.h>

-#include <filesystem>
+//#include <filesystem>

 using namespace std;

@@ -89,24 +89,28 @@ void TextSubtitlesRenderFT::loadFontMap()
     fileList.insert(fileList.end(), fileList1.begin(), fileList1.end());
 #endif

-    for (auto& fontFile : fileList)
+    for (int i = 0; i < fileList.size(); ++i)
     {
         // LTRACE(LT_INFO, 2, "before loading font " << fileList[i].c_str());
-        if (strEndWith(fontFile, "AppleMyungjo.ttf"))
+        if (strEndWith(fileList[i], "AppleMyungjo.ttf"))
             continue;

         FT_Face font;
-        int error = FT_New_Face(library, fontFile.c_str(), 0, &font);
+        int error = FT_New_Face(library, fileList[i].c_str(), 0, &font);
         if (error == 0)
         {
             string fontFamily = strToLowerCase(font->family_name);

             std::map<std::string, std::string>::iterator itr = m_fontNameToFile.find(fontFamily);
+            if (itr == m_fontNameToFile.end())
+                            m_fontNameToFile[fontFamily] = fileList[i];
+            else if (fileList[i].length() < itr->second.length())
+                            m_fontNameToFile[fontFamily] = fileList[i];

-            if (itr == m_fontNameToFile.end() || fontFile.length() < itr->second.length())
+/*            if (itr == m_fontNameToFile.end() || fontFile.length() < itr->second.length())
             {
-                m_fontNameToFile[fontFamily] = std::filesystem::canonical(fontFile).string();
-            }
+                //m_fontNameToFile[fontFamily] = std::filesystem::canonical(fontFile).string();
+            }*/
             FT_Done_Face(font);
         }
         // LTRACE(LT_INFO, 2, "after loading font " << fileList[i].c_str());

mostly because i am not really familiar with c++

but this is only one file, so failing whole build over two lines is nit very nice..

also, linking need "-lm -lstc++" for me - may be cmake can check for this automatically?

jcdr428 commented 2 years ago

Hi @Randrianasulu , maybe you can use boost filesystem::experimental instead of std::filesystem on C++11.

Randrianasulu commented 2 years ago

well, I personally can use any kind of hack - I just thought knowing upstream views on this might be helpful...

anaway, thanks for your work - I think even as is tsmuxer is great (ffmpeg still not quite here when it comes to BD muxing). I tried to integrate tsmuxer into cinelerra-gg, but so far we build for Linux and BSDvs (experimentally, with out-of-tree patches ir manual patching.). 2 years ago there was working way to build Cin-gg under cygwin, but not sure if it still works...

if you have time for this I can point you (or others) to patches and answer qeustions about our somewhat temperamental program (I mostly hack on buildsystem lately, but with goal of integrating all our os support patches into main codebase)

On Sunday, April 17, 2022, jcdr428 @.***> wrote:

Hi @Randrianasulu https://github.com/Randrianasulu , maybe you can use boost filesystem::experimental instead of std::filesystem on C++11.

— Reply to this email directly, view it on GitHub https://github.com/justdan96/tsMuxer/issues/577#issuecomment-1100827968, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJSS7TMM7CFURPY5LUAWWPTVFPBAXANCNFSM5TOZNB6A . You are receiving this because you were mentioned.Message ID: @.***>

qyot27 commented 2 years ago

Hi @Randrianasulu , maybe you can use boost filesystem::experimental instead of std::filesystem on C++11.

The solution we used when getting AviSynth+ building on systems that lacked an easy way to use a C++17 compiler¹ or that ship with a default setup that lacks C++17 filesystem support in libc++², was to use https://github.com/gulrak/filesystem as a submodule, and just conditionally enable std::filesystem through that on setups that couldn't do it the proper way. At least it's not overkill like hooking into Boost is.

¹mostly PPC versions of OS X, as Tigerbrew only gets up to GCC 7, and MacPorts would have taken entirely too long to get anything built...one of these days I'll get around to it, though.

²anything before 10.15, if you want to stick to the default AppleClang. I also vaguely seem to remember one of the *BSDs having an older system libc++ that required installing a newer alternate compiler.

Randrianasulu commented 2 years ago

@qyot27, thanks a lot, will try to add this to default clean tsmuxer sources and see how it goes!

Randrianasulu commented 2 years ago

@qyot27, @jcdr428

i tried to add github.com/gulrak/filesystem to my build like this

guest@slax:~/tsMuxer/build$ cat ../ghc_filesystem.diff
diff --git a/tsMuxer/osdep/textSubtitlesRenderFT.cpp b/tsMuxer/osdep/textSubtitlesRenderFT.cpp
index 198fceb..5e3b22a 100644
--- a/tsMuxer/osdep/textSubtitlesRenderFT.cpp
+++ b/tsMuxer/osdep/textSubtitlesRenderFT.cpp
@@ -27,9 +27,10 @@ const static char FONT_ROOT[] = "/System/Library/Fonts/";
 #include <freetype/ftstroke.h>
 #include <fs/systemlog.h>

-#include <filesystem>
+#include "filesystem.hpp"

 using namespace std;
+using namespace ghc::filesystem;

 namespace text_subtitles
 {
@@ -105,7 +106,7 @@ void TextSubtitlesRenderFT::loadFontMap()

             if (itr == m_fontNameToFile.end() || fontFile.length() < itr->second.length())
             {
-                m_fontNameToFile[fontFamily] = std::filesystem::canonical(fontFile).string();
+                m_fontNameToFile[fontFamily] = ghc::filesystem::canonical(fontFile).string();
             }
             FT_Done_Face(font);
         }
guest@slax:~/tsMuxer/build$

and it produced x86 executable (not yet tested with real muxing)

justdan96 commented 2 years ago

I think I've seen other projects try to detect which filesystem header and namespace to use, I'll have a look if CMake can do that for us.

Edit: as a first step I will just add some lines to the CMake files to ensure that we are alerting the user early on if std::filesystem isn't available.

justdan96 commented 2 years ago

Edit: #588 is the new resolution, removing <filesystem> altogether.

lighterowl commented 2 years ago

588 is now merged so <filesystem> is not used at all, thus closing.