Skip to content
Browse files

several fixes for ofxThreadedImageLoader + ofURLFileLoader

  • Loading branch information...
1 parent 942820a commit 692a45790ba00ba8d2f0199703ce429d1e6ba6ff @arturoc arturoc committed Feb 7, 2013
View
168 addons/ofxThreadedImageLoader/src/ofxThreadedImageLoader.cpp
@@ -3,51 +3,46 @@
ofxThreadedImageLoader::ofxThreadedImageLoader()
:ofThread()
{
- num_loading = 0;
+ nextID = 0;
ofAddListener(ofEvents().update, this, &ofxThreadedImageLoader::update);
- ofRegisterURLNotification(this);
+ ofAddListener(ofURLResponseEvent(),this,&ofxThreadedImageLoader::urlResponse);
startThread();
lastUpdate = 0;
- errorCounter = 0;
+}
+
+ofxThreadedImageLoader::~ofxThreadedImageLoader(){
+ condition.signal();
+ ofRemoveListener(ofEvents().update, this, &ofxThreadedImageLoader::update);
+ ofRemoveListener(ofURLResponseEvent(),this,&ofxThreadedImageLoader::urlResponse);
}
// Load an image from disk.
//--------------------------------------------------------------
-void ofxThreadedImageLoader::loadFromDisk(ofImage* image, string filename) {
-
-
- num_loading++;
+void ofxThreadedImageLoader::loadFromDisk(ofImage& image, string filename) {
+ nextID++;
ofImageLoaderEntry entry(image, OF_LOAD_FROM_DISK);
entry.filename = filename;
- entry.id = num_loading;
+ entry.id = nextID;
entry.image->setUseTexture(false);
entry.name = filename;
lock();
images_to_load_buffer.push_back(entry);
condition.signal();
unlock();
-
-
}
// Load an url asynchronously from an url.
-// * 5 bucks this doesn't work
//--------------------------------------------------------------
-void ofxThreadedImageLoader::loadFromURL(ofImage* image, string url) {
-
-
- num_loading++;
+void ofxThreadedImageLoader::loadFromURL(ofImage& image, string url) {
+ nextID++;
ofImageLoaderEntry entry(image, OF_LOAD_FROM_URL);
entry.url = url;
- entry.id = num_loading;
+ entry.id = nextID;
entry.image->setUseTexture(false);
-
- stringstream ss;
- ss << "image" << entry.id;
- entry.name = ss.str();
+ entry.name = "image" + ofToString(entry.id);
lock();
images_to_load_buffer.push_back(entry);
@@ -59,79 +54,40 @@ void ofxThreadedImageLoader::loadFromURL(ofImage* image, string url) {
// Reads from the queue and loads new images.
//--------------------------------------------------------------
void ofxThreadedImageLoader::threadedFunction() {
-
- errorCounter = 0;
+ deque<ofImageLoaderEntry> images_to_load;
-
while( isThreadRunning() ) {
-
- try {
- lock();
- // wake every 1/2 second just in case we missed something
- condition.tryWait(this->mutex, 500);
- images_to_load.insert( images_to_load.end(),
- images_to_load_buffer.begin(),
- images_to_load_buffer.end() );
-
- images_to_load_buffer.clear();
-
- } catch (exception &e) {
-
- unlock();
-
- stringstream ss;
- ss << "Exception caught in ofxThreadedImageLoader: " << e.what() << endl;
- ofLog( OF_LOG_ERROR, ss.str() );
-
- ++errorCounter;
- // don't overload log
- sleep(errorCounter * 1000);
-
- continue;
- }
+ lock();
+ if(images_to_load_buffer.empty()) condition.wait(mutex);
+ images_to_load.insert( images_to_load.end(),
+ images_to_load_buffer.begin(),
+ images_to_load_buffer.end() );
- unlock();
+ images_to_load_buffer.clear();
+ unlock();
- while( shouldLoadImages() ) {
-
- ofImageLoaderEntry entry = getNextImageToLoad();
- if(entry.image == NULL) {
- continue;
- }
+ while( !images_to_load.empty() ) {
+ ofImageLoaderEntry & entry = images_to_load.front();
if(entry.type == OF_LOAD_FROM_DISK) {
if(! entry.image->loadImage(entry.filename) ) {
- stringstream ss;
- ss << "ofxThreadedImageLoader error loading image " << entry.filename;
- ofLog(OF_LOG_ERROR, ss.str() );
+ ofLogError() << "ofxThreadedImageLoader error loading image " << entry.filename;
}
lock();
images_to_update.push_back(entry);
unlock();
- //cout << "loaded from disk " << entry.name << endl;
- }
- else if(entry.type == OF_LOAD_FROM_URL) {
+ }else if(entry.type == OF_LOAD_FROM_URL) {
lock();
images_async_loading.push_back(entry);
unlock();
ofLoadURLAsync(entry.url, entry.name);
}
-
- // also check while looping so we can continue if possible
- lock();
- images_to_load.insert( images_to_load.end(),
- images_to_load_buffer.begin(),
- images_to_load_buffer.end() );
-
- images_to_load_buffer.clear();
- unlock();
+ images_to_load.pop_front();
}
-
- errorCounter = 0;
}
}
@@ -142,35 +98,27 @@ void ofxThreadedImageLoader::threadedFunction() {
//--------------------------------------------------------------
void ofxThreadedImageLoader::urlResponse(ofHttpResponse & response) {
if(response.status == 200) {
-
lock();
// Get the loaded url from the async queue and move it into the update queue.
entry_iterator it = getEntryFromAsyncQueue(response.request.name);
if(it != images_async_loading.end()) {
(*it).image->loadImage(response.data);
-
+ images_to_update.push_back(*it);
images_async_loading.erase(it);
- //cout << "loaded " << it->name << endl;
}
unlock();
- }
- else {
+ }else{
// log error.
- stringstream ss;
- ss << "Could not image from url, response status: " << response.status;
- ofLog(OF_LOG_ERROR, ss.str());
-
+ ofLogError()<< "Could not get image from url, response status: " << response.status;
+ ofRemoveURLRequest(response.request.getID());
// remove the entry from the queue
lock();
entry_iterator it = getEntryFromAsyncQueue(response.request.name);
if(it != images_async_loading.end()) {
images_async_loading.erase(it);
}
- else {
- ofLog(OF_LOG_WARNING, "Cannot find image in load-from-url queue");
- }
unlock();
}
}
@@ -180,12 +128,12 @@ void ofxThreadedImageLoader::urlResponse(ofHttpResponse & response) {
//--------------------------------------------------------------
void ofxThreadedImageLoader::update(ofEventArgs & a){
- // TODO put a max on the number of images we copy over
- // so that we don't slow thngs down
+ // Load 1 image per update so we don't block the gl thread for too long
lock();
- ofImageLoaderEntry entry = getNextImageToUpdate();
- while (entry.image != NULL) {
+ if (!images_to_update.empty()) {
+
+ ofImageLoaderEntry entry = images_to_update.front();
const ofPixels& pix = entry.image->getPixelsRef();
entry.image->getTextureReference().allocate(
@@ -196,9 +144,8 @@ void ofxThreadedImageLoader::update(ofEventArgs & a){
entry.image->setUseTexture(true);
entry.image->update();
-
- //cout << "Updated " << entry.name << endl;
- entry = getNextImageToUpdate();
+
+ images_to_update.pop_front();
}
unlock();
}
@@ -209,47 +156,10 @@ void ofxThreadedImageLoader::update(ofEventArgs & a){
//--------------------------------------------------------------
ofxThreadedImageLoader::entry_iterator ofxThreadedImageLoader::getEntryFromAsyncQueue(string name) {
entry_iterator it = images_async_loading.begin();
- while(it != images_async_loading.end()) {
+ for(;it != images_async_loading.end();it++) {
if((*it).name == name) {
return it;
}
}
return images_async_loading.end();
}
-
-
-// Pick an entry from the queue with images for which the texture
-// needs to be update.
-// * private, no lock protection, is private function
-//--------------------------------------------------------------
-ofxThreadedImageLoader::ofImageLoaderEntry ofxThreadedImageLoader::getNextImageToUpdate() {
-
- ofImageLoaderEntry entry;
- if(images_to_update.size() > 0) {
- entry = images_to_update.front();
- images_to_update.pop_front();
- }
- return entry;
-}
-
-// Pick the next image to load from disk.
-// * private, no lock protection, is private function
-//--------------------------------------------------------------
-ofxThreadedImageLoader::ofImageLoaderEntry ofxThreadedImageLoader::getNextImageToLoad() {
-
- ofImageLoaderEntry entry;
- if(images_to_load.size() > 0) {
- entry = images_to_load.front();
- images_to_load.pop_front();
- }
- return entry;
-}
-
-// Check if there are still images in the queue.
-// * private, no lock protection, is private function
-//--------------------------------------------------------------
-bool ofxThreadedImageLoader::shouldLoadImages() {
- bool temp = images_to_load.size() > 0;
-
- return temp;
-}
View
69 addons/ofxThreadedImageLoader/src/ofxThreadedImageLoader.h
@@ -9,17 +9,23 @@
// must use poco condition not lock for this
#include "Poco/Condition.h"
-// TODO make this a singleton to prevent unecessary number of threads
-// TODO currently if a large number of images are queued, the update()
-// callback can take a long period while images are updated.
-// in the future a cap on image updates per update() could be implemented
-
using namespace std;
class ofxThreadedImageLoader : public ofThread {
public:
-
-
+ ofxThreadedImageLoader();
+ ~ofxThreadedImageLoader();
+
+ void loadFromDisk(ofImage& image, string file);
+ void loadFromURL(ofImage& image, string url);
+
+
+
+private:
+ void update(ofEventArgs & a);
+ virtual void threadedFunction();
+ void urlResponse(ofHttpResponse & response);
+
// Where to load form?
enum ofLoaderType {
OF_LOAD_FROM_DISK
@@ -31,11 +37,14 @@ class ofxThreadedImageLoader : public ofThread {
struct ofImageLoaderEntry {
ofImageLoaderEntry() {
image = NULL;
+ type = OF_LOAD_FROM_DISK;
+ id=0;
}
- ofImageLoaderEntry(ofImage* pImage, ofLoaderType nType) {
- image = pImage;
+ ofImageLoaderEntry(ofImage & pImage, ofLoaderType nType) {
+ image = &pImage;
type = nType;
+ id=0;
}
ofImage* image;
ofLoaderType type;
@@ -44,42 +53,20 @@ class ofxThreadedImageLoader : public ofThread {
string name;
int id;
};
-
+
+
typedef deque<ofImageLoaderEntry>::iterator entry_iterator;
-
-
- ofxThreadedImageLoader();
- void loadFromDisk(ofImage* image, string file);
- void loadFromURL(ofImage* image, string url);
-
- void update(ofEventArgs & a);
-
- virtual void threadedFunction();
-
- void urlResponse(ofHttpResponse & response);
-
+ entry_iterator getEntryFromAsyncQueue(string name);
+
+ int nextID;
+
+ Poco::Condition condition;
+
+ int lastUpdate;
deque<ofImageLoaderEntry> images_async_loading; // keeps track of images which are loading async
-
- deque<ofImageLoaderEntry> images_to_load;
deque<ofImageLoaderEntry> images_to_load_buffer;
-
-
deque<ofImageLoaderEntry> images_to_update;
-
-private:
-
- bool shouldLoadImages();
- ofImageLoaderEntry getNextImageToLoad();
- ofImageLoaderEntry getNextImageToUpdate();
- entry_iterator getEntryFromAsyncQueue(string name);
-
- int num_loading;
-
- Poco::Condition condition;
-
- int lastUpdate;
-
- int errorCounter;
};
+
View
28 examples/addons/threadedImageLoaderExample/src/testApp.cpp
@@ -5,21 +5,11 @@ void testApp::setup(){
ofSetLogLevel(OF_LOG_VERBOSE);
ofSetVerticalSync(true);
total = 24;
-
- for(int i = 0; i < total; ++i) {
- ofImage* img = new ofImage();
- images.push_back(img);
- loader.loadFromDisk(img, "of" + ofToString(i) + ".png");
-
- ofImage* url_img = new ofImage();
- images.push_back(url_img);
- loader.loadFromURL(images.back(), "http://www.roxlu.com/assets/images/of_inverted.png");
+ images.resize(total*2);
+ for(int i = 0; i < total; ++i) {
+ loader.loadFromDisk(images[i*2], "of" + ofToString(i) + ".png");
+ loader.loadFromURL(images[i*2+1], "http://www.openframeworks.cc/of_inverted.png");
}
-
- //TODO: no! this should be internal - overloading << doesn't make sense
- //cout << loader << endl;
-
- loader.startThread(false, false);
}
//--------------------------------------------------------------
@@ -31,17 +21,17 @@ void testApp::update(){
void testApp::draw(){
// draw the images.
- glColor3f(1,1,1);
- for(int i = 0; i < images.size(); ++i) {
+ ofSetColor(255);
+ for(int i = 0; i < (int)images.size(); ++i) {
int x = (i%8);
int y = (i/8);
- images.at(i)->draw(x*128,y*128, 128,128);
+ images[i].draw(x*128,y*128, 128,128);
}
// draw the FPS
- glColor3f(1,1,1);
ofRect(0,ofGetHeight()-20,30,20);
- glColor3f(0,0,0);
+
+ ofSetColor(0);
ofDrawBitmapString(ofToString(ofGetFrameRate(),0),5,ofGetHeight()-5);
}
View
2 examples/addons/threadedImageLoaderExample/src/testApp.h
@@ -21,7 +21,7 @@ class testApp : public ofBaseApp{
void gotMessage(ofMessage msg);
ofxThreadedImageLoader loader;
- vector<ofImage*> images;
+ vector<ofImage> images;
int total;
};
View
34 libs/openFrameworks/utils/ofURLFileLoader.cpp
@@ -28,15 +28,18 @@ using Poco::Exception;
static bool factoryLoaded = false;
int ofHttpRequest::nextID = 0;
-ofEvent<ofHttpResponse> ofURLResponseEvent;
+ofEvent<ofHttpResponse> & ofURLResponseEvent(){
+ static ofEvent<ofHttpResponse> * event = new ofEvent<ofHttpResponse>;
+ return *event;
+}
ofURLFileLoader::ofURLFileLoader() {
if(!factoryLoaded){
try {
HTTPStreamFactory::registerFactory();
factoryLoaded = true;
}
- catch (Poco::SystemException PS) {
+ catch (Poco::SystemException & PS) {
ofLog(OF_LOG_ERROR, "Got exception in url ofURLFileloader");
}
}
@@ -74,26 +77,26 @@ int ofURLFileLoader::saveAsync(string url, string path){
}
void ofURLFileLoader::remove(int id){
- lock();
+ Poco::ScopedLock<ofMutex> lock(mutex);
for(int i=0;i<(int)requests.size();i++){
if(requests[i].getID()==id){
requests.erase(requests.begin()+i);
return;
}
}
- unlock();
+ ofLogError() << "trying to remove request " << id << " not found";
}
void ofURLFileLoader::clear(){
- lock();
+ Poco::ScopedLock<ofMutex> lock(mutex);
requests.clear();
while(!responses.empty()) responses.pop();
- unlock();
}
void ofURLFileLoader::start() {
if (isThreadRunning() == false){
- startThread(false, false); // blocking, verbose
+ ofAddListener(ofEvents().update,this,&ofURLFileLoader::update);
+ startThread(true, false); // blocking, verbose
}else{
ofLog(OF_LOG_VERBOSE,"signaling new request condition");
condition.signal();
@@ -115,8 +118,8 @@ void ofURLFileLoader::threadedFunction() {
ofHttpResponse response(handleRequest(request));
+ lock();
if(response.status!=-1){
- lock();
// double-check that the request hasn't been removed from the queue
if( (requests.size()==0) || (requests.front().getID()!=request.getID()) ){
// this request has been removed from the queue
@@ -126,13 +129,12 @@ void ofURLFileLoader::threadedFunction() {
ofLog(OF_LOG_VERBOSE,"got response to request " + requests.front().name + " status "+ofToString(response.status) );
responses.push(response);
requests.pop_front();
- ofAddListener(ofEvents().update,this,&ofURLFileLoader::update);
}
- unlock();
}else{
+ responses.push(response);
ofLog(OF_LOG_VERBOSE,"failed getting request " + requests.front().name);
}
- ofSleepMillis(10);
+ unlock();
}else{
ofLog(OF_LOG_VERBOSE,"stopping on no requests condition");
condition.wait(mutex);
@@ -180,21 +182,21 @@ ofHttpResponse ofURLFileLoader::handleRequest(ofHttpRequest request) {
return ofHttpResponse(request,-1,"ofURLFileLoader fatal error, couldn't catch Exception");
}
+ return ofHttpResponse(request,-1,"ofURLFileLoader fatal error, couldn't catch Exception");
}
void ofURLFileLoader::update(ofEventArgs & args){
lock();
- if(responses.size()){
+ while(!responses.empty()){
ofHttpResponse response(responses.front());
ofLog(OF_LOG_VERBOSE,"ofURLLoader::update: new response " +response.request.name);
responses.pop();
unlock();
- ofNotifyEvent(ofURLResponseEvent,response);
- }else{
- ofRemoveListener(ofEvents().update,this,&ofURLFileLoader::update);
- unlock();
+ ofNotifyEvent(ofURLResponseEvent(),response);
+ lock();
}
+ unlock();
}
View
6 libs/openFrameworks/utils/ofURLFileLoader.h
@@ -62,16 +62,16 @@ int ofSaveURLAsync(string url, string path);
void ofRemoveURLRequest(int id);
void ofRemoveAllURLRequests();
-extern ofEvent<ofHttpResponse> ofURLResponseEvent;
+ofEvent<ofHttpResponse> & ofURLResponseEvent();
template<class T>
void ofRegisterURLNotification(T * obj){
- ofAddListener(ofURLResponseEvent,obj,&T::urlResponse);
+ ofAddListener(ofURLResponseEvent(),obj,&T::urlResponse);
}
template<class T>
void ofUnregisterURLNotification(T * obj){
- ofRemoveListener(ofURLResponseEvent,obj,&T::urlResponse);
+ ofRemoveListener(ofURLResponseEvent(),obj,&T::urlResponse);
}

0 comments on commit 692a457

Please sign in to comment.
Something went wrong with that request. Please try again.