ofVideoPlayer constructor with filename

[dimitre]

so this is now possible:

ofVideoPlayer video { "video.mov" };

in the likes of ofImage and ofxSvg

[ofTheo]

hmm the msys2 error seems new - error linking glfw3dll

D:/a/_temp/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/13.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: cannot find -lglfw3dll: No such file or directory

any idea @oxillo ?

[dimitre]

Ok, finally msys2 tests are fixed, now it passes tests. any opinion on this one @ofTheo @artificiel ?

[artificiel]

one thing to reduce code duplication is to call a delegate constructor from the specialized one:

ofVideoPlayer::ofVideoPlayer(const of::filesystem::path & fileName): ofVideoPlayer() {
	// FIXME: Convert internally everything to FS
	load(ofPathToString(fileName));
}

also, is there a test that accompanies this new constructor? I'm all for constructors, but their immediate execution implies all dependencies are properly init and setup — to ensure that there are no "unrealized dependencies" at construction it would be good to include a test as there are many flavours of the actual VideoPlayers/platforms/etc.

[dimitre]

Thanks, good idea. updated with the constructor.
Yes agree about tests, but I think we can merge without having all the tests as it is an alternative way of initializing the object, not the main one covered in examples, etc.

[artificiel]

what I mean is that the constructor will be executed early in the app lifecycle, before ofMainLoop::run() and there has been some segfaults issues with constructing things depending on lower-level ressources tied to the interaction with the OS (ALSA on linux for example, and I think some openGL is note quite ready by then) so if the VideoPlayer requires some of that it may not be ready by construction time (but will be by setup() time). these things might be platform-specific.

"normal" (non-dependent) objects are not a problem, but I'm raising the point on ofVideoPlayer as there are many underlying engines on the different platforms and an edge case bug can be difficult to debug later when the timing correlation between "code change" and "bug report" is not tight.

[dimitre]

I've now tested in macOS and Linux, working great on both.
I've used just for testing the videoPlayerExample

ofVideoPlayer fingerMovie { "movies/fingers.mp4" }; 

it won't work in a extreme case like putting it inside main.cpp, but ofImage won't work either.

[artificiel]

great that it works! I am all for empowering the declaration side.

constructors are controversial because they overlap low-level allocation with configuration, and it adds "one other way" of doing things. I synthesized the discussion here with a "guideline proposal": #1562 (comment) (those guidelines are of course part of a discussion, but considering the philosophical/technical overlap, it's good to agree on some references to inform the OF API as a whole).

the core of the matter is that OF-users-facing constructors should be consistent and not open "new territory". the point 4 of that list goes with the idea that if you know the constructor syntax you know the setup syntax (and vice-versa, up to the limits of the constructors). of course more things may happen at construction (in regards to allocation etc), and some things might only be able in setup(), but from the user perspective, it's "one logical set of arguments" to learn.

in that spirit, it would be inconsistent to have the constructor do user-things not available in setup(). beyond pattern consistency, the idea is also to have a "designated initializer" method (DRY => no multiple copies of similar code).

ofVideoPlayer has not setup(); I thus suggest that setup method be added, that executes the same as load(), and is passed in the constructor. also I would enjoy a bool argument to specify autoplay, as it's a common setting where both cases are interesting to have (and with that you can determine in the constructor if the movie is to play at once, or is just pre-loaded):

ofVideoPlayer::setup(fs:path path, bool autoplay=true) {
  load(path);
  if (autoplay) play();
}
ofVideoPlayer::ofVideoPlayer (fs:path path, bool autoplay=true) {
  setup(path, autoplay);
}

[artificiel]

i re-read the thread I linked above and notice Arturo mentions setup/load/allocate as "constructor-compatible". but I wonder about that trifecta — for instance ofImage has both load and allocate... of course the params are pretty different so we could have a "load-following" constructor as well as a "allocate-following" constructor, but it means 2 possible syntaxes in the declaration, with implicit conversion on unnamed arguments... could get tricky.

i am not sure of the historical division between load/setup/allocate --> is setup sort of "what is not load nor allocate"?

i tend to see it from the user functional perspective "I need to setup this object", and setup might mean allocation or loading or whatever... ultimately loading and allocating are about "setting up". of course RAM allocation has a specific weight, and "load" is clearly a directive (that will in turn allocate), but they also are about "setting up"... since the object in not useable without it.

[artificiel]

ofFbo is a tricky one: it has ofFboSettings (where "Settings" sounds like "setup"), but they are to be passed in allocate()...

myFbo.setup(myFBOSettings); sounds reasonable... and in a way more precise, as myFbo.allocate(myFBOSettings); does much more than allocating...

anyway I don't want to re-question everything, but I wish for the most wide and constant constructor support across the OF API, hence figuring out guidelines and terminology is key to user acquiring patterns that can by applied intuitively.