Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let F3D send it's output image in a stream rather than into a file on disk #1101

Closed
mrdeveloperdude opened this issue Dec 17, 2023 · 11 comments
Assignees
Labels
type:enhancement New feature or request
Milestone

Comments

@mrdeveloperdude
Copy link

See related issue for streaming input.

F3D currently accepts a png filename as a command-line argument for where to store it's output.

F3D will then write the output image into that file after rendering.

Writing the image to disk can incur significant IO penalty. Further, in many cases we won't need the image on disk at all, we simply want the image in memory for processing or redirection over a network or into an external storage system.

This feature request is for having a sanctified and documented way of streaming the image data out of F3D without having to save it to disk first to avoid said IO penalty.

There are ways to possibly do this (untested by issue author) by the use of tmpfs on linux platforms, and by passing /dev/stdout as the image filename.

There is also the possibility of using libf3d instead of the stand alone executable.

Here are some relevant comments from discord:

stream input is on the to-do list but requires changes in underlying libs so the ETA isn't entirely up to F3D (the changes are slowly but surely making their way into VTK as far as I know)
stream output should be doable on F3D's side

tracks that can be investigated without waiting for any new features:
using a ram disk, I'd be interested in seeing a benchmark of piping from stdout to stdin vs. writing and reading in a tmpfs directory (in theory piping should be better because it allows actual streaming, however in the case at hand we can't really render without buffering the whole input model and output image anyway so the streaming aspect won't matter and we just want fast/cheap IO)
using libf3d instead of the F3D application, for example through the python bindings, would let you read rendered images from memory and also allows to do multiple renders of the same model without reloading it

@mwestphal
Copy link
Contributor

This is already supported by the libf3d, but there is no mechanism to expose that in the F3D application yet.

The first question would be to identify how to do that in a cross platform way.

@mwestphal mwestphal added the type:enhancement New feature or request label Dec 17, 2023
@Meakk
Copy link
Member

Meakk commented Dec 20, 2023

I confirm --output /dev/stdout works as intended.

@snoyer
Copy link
Contributor

snoyer commented Dec 21, 2023

As @mwestphal pointed out, @Meakk's newly merged image::saveBuffer can be used for this feature.

Here's a quick and dirty patch that could be a start for someone feeling like doing a proper PR:

diff --git a/application/F3DStarter.cxx b/application/F3DStarter.cxx
index 43a326c6..876b7f61 100644
--- a/application/F3DStarter.cxx
+++ b/application/F3DStarter.cxx
@@ -14,6 +14,7 @@
 
 #include <cassert>
 #include <filesystem>
+#include <iostream>
 #include <set>
 
 namespace fs = std::filesystem;
@@ -163,8 +164,16 @@ int F3DStarter::Start(int argc, char** argv)
   this->Internals->Parser.GetOptions(
     this->Internals->AppOptions, this->Internals->DynamicOptions, files);
 
-  // Set verbosity level early from command line
-  F3DInternals::SetVerboseLevel(this->Internals->AppOptions.VerboseLevel);
+  const bool renderToStdout = this->Internals->AppOptions.Output == "-";
+  if (renderToStdout)
+  {
+    f3d::log::setVerboseLevel(f3d::log::VerboseLevel::ERROR);
+  }
+  else
+  {
+    // Set verbosity level early from command line
+    F3DInternals::SetVerboseLevel(this->Internals->AppOptions.VerboseLevel);
+  }
 
   // Load plugins from the app options
   this->Internals->Parser.LoadPlugins(this->Internals->AppOptions);
@@ -181,7 +190,10 @@ int F3DStarter::Start(int argc, char** argv)
       this->Internals->AppOptions, this->Internals->DynamicOptions, files);
 
     // Set verbosity level again if it was defined in the configuration file global block
-    F3DInternals::SetVerboseLevel(this->Internals->AppOptions.VerboseLevel);
+    if (!renderToStdout)
+    {
+      F3DInternals::SetVerboseLevel(this->Internals->AppOptions.VerboseLevel);
+    }
   }
 
 #if __APPLE__
@@ -407,7 +419,15 @@ int F3DStarter::Start(int argc, char** argv)
       }
 
       f3d::image img = window.renderToImage(this->Internals->AppOptions.NoBackground);
-      img.save(this->Internals->AppOptions.Output);
+      if (renderToStdout)
+      {
+        const auto buffer = img.saveBuffer();
+        std::copy(buffer.begin(), buffer.end(), std::ostreambuf_iterator(std::cout));
+      }
+      else
+      {
+        img.save(this->Internals->AppOptions.Output);
+      }
     }
     // Start interaction
     else

@Meakk
Copy link
Member

Meakk commented Dec 21, 2023

Is this - standard? I don't remember I've seen that in the past.
@snoyer PR? 😛

@snoyer
Copy link
Contributor

snoyer commented Dec 21, 2023

Is this - standard? I don't remember I've seen that in the past

I don't know about standard but having - as a special case seems pretty accepted. Off the top of my head cat and curl use it for stdin/stdout; git and cd have their own meanings for - too but I wouldn't be able to tell you what they are.

@snoyer PR? 😛

Could slap a "good first issue" tag on it and let someone else have a go

@mwestphal
Copy link
Contributor

I don't know about standard but having - as a special case seems pretty accepted.

Do you have a good source for this ?

@mwestphal
Copy link
Contributor

Thanks for digging that up. It is very linuxy though. We need to make sure this will have no impact and work as intended with macOS and Windows. Can - be a file on Windows ?

@snoyer
Copy link
Contributor

snoyer commented Dec 21, 2023

I don't know about Windows but - can be a file on linux, if you wanted to write to it instead of stdout you'd need to put a more explicit path to point at it (eg. ./-, ~/-, /foo/bar/-, ...)

If this - idea is too linuxy for F3D it would only be a matter of deciding an alternative syntax (eg --output=stdout or whatever, with the same gotcha that that could be a file too) or a dedicated argument altogether (something like --pipe-output maybe?)

@mwestphal
Copy link
Contributor

I'm not against the idea tbh, even if it is linuxy. I'm just trying to check for unforseen consequences.

@mwestphal mwestphal added the help wanted Please help with this issue! label Jan 7, 2024
@mwestphal mwestphal added workflow:investigation and removed help wanted Please help with this issue! labels Jan 26, 2024
@mwestphal mwestphal added this to F3D Feb 3, 2024
@mwestphal mwestphal added this to the 2.4.0 milestone Feb 4, 2024
@mwestphal mwestphal moved this to Investigate in F3D Feb 4, 2024
@mwestphal mwestphal moved this from Investigate to To do in F3D Feb 9, 2024
@Meakk Meakk self-assigned this Mar 22, 2024
@Meakk Meakk moved this from To do to In progress in F3D Mar 22, 2024
@Meakk Meakk moved this from In progress to In review in F3D Mar 22, 2024
@github-project-automation github-project-automation bot moved this from In review to Done in F3D Mar 24, 2024
@mwestphal
Copy link
Contributor

@mrdeveloperdude the last nightly release contains that feature, use - as an output file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

4 participants