From 5dcb9cf343e98303546feed574d85ffd277f6aa3 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Mon, 19 Nov 2007 04:58:18 +0100 Subject: [PATCH] WIP implemented basic asset dependencies, > todo: actually setup dependencies in the various ctors, fix the broken test! --- src/common/util.hpp | 12 +++++++ src/proc/asset.cpp | 60 ++++++++++++++++++------------- src/proc/asset.hpp | 21 +++++++---- src/proc/asset/clip.cpp | 5 +-- src/proc/asset/media.cpp | 13 +++++-- src/proc/asset/media.hpp | 12 +++++-- src/proc/asset/preview.cpp | 48 +++++++++++++++++++++++++ src/proc/asset/preview.hpp | 3 +- src/proc/asset/unknown.cpp | 9 ++++- src/proc/asset/unknown.hpp | 2 +- src/proc/assetmanager.cpp | 1 + src/proc/mobject/session/clip.cpp | 4 +-- src/proc/mobject/session/clip.hpp | 2 -- wiki/renderengine.html | 10 +++--- 14 files changed, 153 insertions(+), 49 deletions(-) diff --git a/src/common/util.hpp b/src/common/util.hpp index 3d465f666..ea21c2a9c 100644 --- a/src/common/util.hpp +++ b/src/common/util.hpp @@ -102,6 +102,18 @@ namespace util return end != std::find(cont.begin(),end, val); } + /** shortcut for removing all copies of an Element + * in any sequencial collection */ + template + inline typename SEQ::iterator + removeall (SEQ& coll, typename SEQ::value_type& val) + { + typename SEQ::iterator collEnd = coll.end(); + return coll.erase (std::remove (coll.begin(), collEnd, val), + collEnd + ); + } + /** shortcut for operating on all elements of a container. * Isn't this already defined somewhere? It's so obvious.. diff --git a/src/proc/asset.cpp b/src/proc/asset.cpp index 4fb8429a8..f9136bb20 100644 --- a/src/proc/asset.cpp +++ b/src/proc/asset.cpp @@ -25,10 +25,17 @@ #include "proc/assetmanager.hpp" #include "common/util.hpp" +#include #include +#include +using boost::bind; using boost::format; +using boost::function; +using util::contains; +using util::removeall; +using util::for_each; using util::cStr; @@ -70,29 +77,8 @@ namespace asset } - /** List of entities this asset depends on or requires to be functional. - * May be empty. The head of this list can be considered the primary prerequisite - */ - vector - Asset::getParents () const - { - UNIMPLEMENTED ("Asset dependencies."); - } - - - /** All the other assets requiring this asset to be functional. - * For example, all the clips depending on a given media file. - * May be empty. The dependency relation is transitive. - */ - vector - Asset::getDependant () const - { - UNIMPLEMENTED ("Asset dependencies."); - } - - /** - * weather this asset is swithced on and consequently included + * whether this asset is swithced on and consequently included * in the fixture and participates in rendering. */ bool @@ -114,18 +100,44 @@ namespace asset } + + + void + Asset::unregister (PAsset& other) ///< @internal + { + other->unlink (this->id); + } + /** release all links to other Asset objects held internally. */ void Asset::unlink () { - UNIMPLEMENTED ("deleting Assets."); + function unregister_me = bind(&Asset::unregister, this,_1); + + for_each (parents, unregister_me); + for_each (dependants, unregister_me); + dependants.clear(); + parents.clear(); } /** variant dropping only the links to the given Asset */ void Asset::unlink (IDA target) { - UNIMPLEMENTED ("deleting Assets."); + PAsset asset (AssetManager::instance().getAsset (target)); + removeall (dependants,asset); + removeall (parents,asset); + } + + + void + Asset::defineDependency (PAsset parent) + { + PAsset p_this (AssetManager::getPtr(*this)); + REQUIRE (!contains (parent->dependants, p_this)); + REQUIRE (!contains (this->parents, parent)); + parents.push_back (parent); + parent->dependants.push_back(p_this); } diff --git a/src/proc/asset.hpp b/src/proc/asset.hpp index a32111bfb..ebac8bd7a 100644 --- a/src/proc/asset.hpp +++ b/src/proc/asset.hpp @@ -104,6 +104,7 @@ namespace asset class Asset; class AssetManager; typedef const ID& IDA; + typedef shared_ptr PAsset; @@ -210,6 +211,9 @@ namespace asset /** user visible qualification of the thing, unit or concept represented by this asset. * perferably "in one line". To be localized. */ const string longDesc; + + vector parents; + vector dependants; @@ -237,7 +241,15 @@ namespace asset * Asset, leaving all other links intact. Usable for propagating */ virtual void unlink (IDA target); + /** establish a connection between this and the given parent asset, + * denoting we are in some way dependant on the parent. */ + virtual void defineDependency (PAsset parent); + friend class AssetManager; + + private: + void unregister (PAsset& other); + @@ -245,15 +257,13 @@ namespace asset /** List of entities this asset depends on or requires to be functional. * May be empty. The head of this list can be considered the primary prerequisite */ - vector > - getParents () const; ////////////////TODO: better const vector, and return a ref! + const vector& getParents () const { return parents; } /** All the other assets requiring this asset to be functional. * For example, all the clips depending on a given media file. * May be empty. The dependency relation is transitive. */ - vector > - getDependant () const; + const vector& getDependant () const { return dependants; } /** weather this asset is swithced on and consequently * included in the fixture and participates in rendering @@ -270,9 +280,6 @@ namespace asset }; - /** shorthand for refcounting Asset pointers */ - typedef shared_ptr PAsset; - /** ordering of Asset smart ptrs based on Ident tuple. * @todo currently supporting only smart_ptr. */ inline bool operator== (const PAsset& a1, const PAsset& a2) { return a1 && a2 && ( 0==a1->ident.compare(a2->ident));} diff --git a/src/proc/asset/clip.cpp b/src/proc/asset/clip.cpp index 9ae562bc8..4de320bf4 100644 --- a/src/proc/asset/clip.cpp +++ b/src/proc/asset/clip.cpp @@ -34,7 +34,7 @@ namespace asset { /** @internal derive a sensible asset ident tuple * when creating a asset::Clip based on some asset::Media - * @todo getting this one is important for handling creation + * @todo getting this one correct is important for handling creation * of multiple clip instances from one media. Means we * have still to figure out a sensible concept... */ @@ -61,7 +61,8 @@ namespace asset Clip::Clip (const Media& mediaref) : Media (createClipIdent (mediaref), - mediaref.getFilename()) + mediaref.getFilename(), + mediaref.getLength()) , source_ (mediaref) , clipMO_ (createClipMO (*this, source_)) { diff --git a/src/proc/asset/media.cpp b/src/proc/asset/media.cpp index 9aa09225f..6d61d7004 100644 --- a/src/proc/asset/media.cpp +++ b/src/proc/asset/media.cpp @@ -105,7 +105,7 @@ namespace asset Media::PProcPatt - Media::howtoProc () + Media::howtoProc () const { UNIMPLEMENTED ("calculate and return processing pattern for media asset"); PProcPatt ppatt; //TODO:null @@ -115,6 +115,13 @@ namespace asset } + cinelerra::Time + Media::getLength() const + { + return len_; + } + + MediaFactory Media::create; ///< storage for the static MediaFactory instance @@ -148,8 +155,10 @@ namespace asset { if (isnil (key.name)) key.name=extractName(file); TODO ("file exists?"); + TODO ("extract media file properties"); + Time length(25); TODO ("detecting and wiring multichannel compound media!"); - pM = new Media (key,file); + pM = new Media (key,file,length); } ASSERT (pM); ENSURE (key.category.hasKind (VIDEO) || key.category.hasKind(AUDIO)); diff --git a/src/proc/asset/media.hpp b/src/proc/asset/media.hpp index 4d066b5c3..defaf51b2 100644 --- a/src/proc/asset/media.hpp +++ b/src/proc/asset/media.hpp @@ -50,6 +50,8 @@ namespace asset class MediaFactory; class ProcPatt; + using cinelerra::Time; + template<> class ID : public ID @@ -67,6 +69,7 @@ namespace asset class Media : public Asset { string filename_; + const Time len_; public: typedef shared_ptr PMedia; @@ -97,10 +100,15 @@ namespace asset * necessary for this Media or Clip. This includes * Codecs and postprocessing (stretching, deinterlacing...) */ - PProcPatt howtoProc (); + PProcPatt howtoProc () const; + + /** @return the overall length of the media represented by this asset */ + virtual Time getLength () const; + protected: - Media (const Asset::Ident& idi, const string& file) : Asset(idi), filename_(file) {} + Media (const Asset::Ident& idi, const string& file, Time length) + : Asset(idi), filename_(file), len_(length) {} friend class MediaFactory; /** get or create the correct asset::Clip diff --git a/src/proc/asset/preview.cpp b/src/proc/asset/preview.cpp index 85dc98596..212e85666 100644 --- a/src/proc/asset/preview.cpp +++ b/src/proc/asset/preview.cpp @@ -25,6 +25,54 @@ namespace asset { + + + namespace + { + /** @internal derive a sensible asset ident tuple when creating + * a proxy placeholder media based on some existing asset::Media + * @todo getting this one right is important for the handling + * of "proxy editing".... + */ + const Asset::Ident + createProxyIdent (const Asset::Ident& mediaref) + { + string name (mediaref.name + "-proxy"); // TODO something sensible here; append number, sanitize etc. + Category category (mediaref.category); + TODO ("put it in another subfolder within the same category??"); + return Asset::Ident (name, category, + mediaref.org, + mediaref.version ); + + } + } + + + + /** create a preview placeholder ("proxy media") for the given + * media asset + */ + Preview::Preview (const Media& mediaref) + : Media (createProxyIdent (mediaref.ident), + mediaref.getFilename(), + mediaref.getLength()) + { + UNIMPLEMENTED ("do something to setup proxy media"); + } + + + + /** create a dummy placeholder + * @internal for use by asset::Unknown + * @todo better design! + */ + Preview::Preview (const Asset::Ident& idi, string name, Time length) + : Media (createProxyIdent (idi), + name, length) + { + TODO ("work out how to handle unknown media placheholder"); + } + diff --git a/src/proc/asset/preview.hpp b/src/proc/asset/preview.hpp index 689bdc295..abf3734f3 100644 --- a/src/proc/asset/preview.hpp +++ b/src/proc/asset/preview.hpp @@ -37,7 +37,8 @@ namespace asset class Preview : public Media { protected: - Preview (const Asset::Ident& idi) : Media(idi,"") {} + Preview (const Media& mediaref); + Preview (const Asset::Ident& idi, string name, Time length); friend class MediaFactory; }; diff --git a/src/proc/asset/unknown.cpp b/src/proc/asset/unknown.cpp index 8e4e558cd..357db430b 100644 --- a/src/proc/asset/unknown.cpp +++ b/src/proc/asset/unknown.cpp @@ -26,8 +26,15 @@ namespace asset { - /** */ + const cinelerra::Time DUMMY_TIME (25); + + /** */ + Unknown::Unknown (const Asset::Ident& idi) + : Preview (idi, "", DUMMY_TIME) + { + TODO ("do something sensible with the »unknown media« placeholder..."); + } } // namespace asset diff --git a/src/proc/asset/unknown.hpp b/src/proc/asset/unknown.hpp index 189a6438b..85db2ac81 100644 --- a/src/proc/asset/unknown.hpp +++ b/src/proc/asset/unknown.hpp @@ -38,7 +38,7 @@ namespace asset class Unknown : public Preview { protected: - Unknown (const Asset::Ident& idi) : Preview(idi) {} + Unknown (const Asset::Ident& idi); friend class MediaFactory; }; diff --git a/src/proc/assetmanager.cpp b/src/proc/assetmanager.cpp index dfbe7c7ce..5fa4183f2 100644 --- a/src/proc/assetmanager.cpp +++ b/src/proc/assetmanager.cpp @@ -246,6 +246,7 @@ namespace asset template shared_ptr AssetManager::getAsset (const ID& id) throw(cinelerra::error::Invalid); template shared_ptr AssetManager::getAsset (const ID& id) throw(cinelerra::error::Invalid); + template shared_ptr AssetManager::getPtr (const Asset& asset); template shared_ptr AssetManager::getPtr (const Media& asset); diff --git a/src/proc/mobject/session/clip.cpp b/src/proc/mobject/session/clip.cpp index c5cba897c..e5e097073 100644 --- a/src/proc/mobject/session/clip.cpp +++ b/src/proc/mobject/session/clip.cpp @@ -57,8 +57,8 @@ namespace mobject void Clip::setupLength() { - UNIMPLEMENTED ("calculate the length of a clip and set length field"); - // will use mediaDef to query media parameters.... + TODO ("really calculate the length of a clip and set length field"); + this->length = mediaDef_.getLength(); } diff --git a/src/proc/mobject/session/clip.hpp b/src/proc/mobject/session/clip.hpp index 9090e347d..ab25f8a76 100644 --- a/src/proc/mobject/session/clip.hpp +++ b/src/proc/mobject/session/clip.hpp @@ -59,8 +59,6 @@ namespace mobject /** startpos in source */ Time start_; - //TODO: where to put the duration ??? - const Media & mediaDef_; const asset::Clip & clipDef_; diff --git a/wiki/renderengine.html b/wiki/renderengine.html index e45fc4aad..83be72225 100644 --- a/wiki/renderengine.html +++ b/wiki/renderengine.html @@ -1440,7 +1440,7 @@ As a //first shot// Ichthyo considers the following approach: [img[Asset Classess|uml/fig130437.png]] -
+
Of course: Cinelerra currently leaks memory and crashes regularilly. For the newly written code, besides retaining the same performance level, a main goal is to use methods and techniques known to support the writing of quality code. So, besides the MultithreadConsiderations, a solid strategy for managing the ownership of allocated memory blocks is necessary right from start.
 
 !Problems
@@ -1448,14 +1448,14 @@ As a //first shot// Ichthyo considers the following approach:
 # Some (not all) parts of the core application are non-deterministic. That means, we can't tie the memory management to any assumptions on behalf of the execution path
 
 !C++ solution
-First of all -- this doesn't concern //every// allocation. It rather means there are certain //dangerous areas// which need to be identified. And as always, instead of carrying the inherent complexities of the problem into the solution, we should rather look for a common solution pattern which helps factoring out the complexity.
+First of all -- this doesn't concern //every// allocation. It rather means there are certain //dangerous areas// which need to be identified. Anyhow, instead of carrying inherent complexities of the problem into the solution, we should rather look for  common solution pattern(s) which help factoring out complexity.
 
-For the case in question this seems to be the ''resource allocation is construction'' pattern. Which boils down to basically never using bare pointers when concerned with ownership. Instead, ownership should be handled by smart-pointers.
+For the case here in question this seems to be the ''resource allocation is construction'' pattern. Which boils down to basically never using bare pointers when concerned with ownership. Instead, ownership should be handled by smart-pointers.
 
 !!usage scenarios
 # __existence is being used__: Objects just live for being referred to in a object network. In this case, use refcounting smart-pointers for every ref. (note: problem with cyclic refs)
 # __entity bound ownership__: Objects can be tied to some long living entity in the program, which holds the smart-pointer
-#* if the existence of these ref-holding entity can be //guaranteed// (like a contract), then the other users can build a object network with conventional pointers
+#* if the existence of these ref-holding entity can be //guaranteed// (as if by contract), then the other users can build a object network with conventional pointers
 #* otherwise, when the ref-holding entity //can disappear// in a regular program state, we need weak-refs and checking (because by our postulate the controlled resource needs to be destructed immediately, otherwise we would have the first case, existence == being used)
 
 !!!dangerous uses
@@ -1470,7 +1470,7 @@ For the case in question this seems to be the ''resource allocation is construct
 * the EDL and the defined [[Asset]]s belong together to one Session. If the Session is closed, this means a internal shutdown of the whole ProcLayer, i.e. closing of all GUI representations and terminating all render processes. If these calles are implemented as blocking operations, we can assert that as long as any GUI representation or any render process is running, there is a valid Session and EDL.
 
 !using Factories
-And, last but not least, doing all actual allocations is the job of the backend. Exceptions being long-lived objects, like the Session or the EDL, which are created once and don't bear the danger of causing memory pressure. Besides that, the ProcLayer code shouldn't issue "new" and "delete", rather it should use some central [[Factories]] for all allocation and freeing, so we can redirect these calls down to the backend, which may use pooling or special placement allocators or the like. The rationale is, for modern hardware/architectures, care has to be taken with heap allocations, esp. with many small objects and irregular usage patterns.
+And, last but not least, doing all actual allocations is the job of the backend. Exceptions being long-lived objects, like the Session or the EDL, which are created once and don't bear the danger of causing memory pressure. Besides, the ProcLayer code shouldn't issue "new" and "delete" when it comes in hand, rather it should use some centralized [[Factories]] for all allocation and freeing, so we can redirect these calls down to the backend, which may use pooling or special placement allocators or the like. The rationale is, for modern hardware/architectures, care has to be taken with heap allocations, esp. with many small objects and irregular usage patterns.