How to synchronize QNetworkAccessManager ?

Gourmand

Me dropped [url=http://developer.qt.nokia.com/forums/viewthread/5196/]attempts assemble Qt statically with WebKit[/url]. I turned to use QNetworkAccessManager instead. But got another problem.

Task: application loads simple text list of image names from some Web-server. Then it has to show and process each image one by one. Quantity of images can be large - tens of thousands, may be even hundreds of thousands. I start this process like this:

@void MainWindow::paintEvent( QPaintEvent * event )
{
if( firstpaint )
{
listmanager->get( QNetworkRequest( QUrl( "http://........" ) ) );
firstpaint = false;
}
}@

sorry, but did not find better place.

Then after list downloading finishes, Qt calls this method:

@void MainWindow::finishedList( QNetworkReply * reply )
{
if( reply->error() == QNetworkReply::NoError )
{
forever
{
QByteArray byteline = reply->readLine();
if( byteline.size() == 0 )
break;
urllist << byteline; // urllist is a QStringList
}
foreach( QString url, urllist )
imgmanager->get( QNetworkRequest( url ) ) );
}
else
networkerror = true;
reply->deleteLater();
}@

I made separate reading cycle for future purposes. And there is of course method for image processing. It has to be called after each image downloaded:

@void MainWindow::finishedImg( QNetworkReply * reply )
{
if( reply->error() == QNetworkReply::NoError )
{
// show and process but doesn't matter how..............
}
else
networkerror = true;
reply->deleteLater();
}@

If I start this with one image - then I see it in MainWindow and processing works. But if list contains a number of images, then call imgmanager->get( QNetworkRequest( url ) ) ); just pushes requests to network queue. First call of MainWindow::finishedImg( QNetworkReply * reply ) appears only after all image requests was sent to queue. They shoot like a Gatling machine gun. If there are lot of images then I get memory overflow.

How to force QNetworkAccessManager process image downloading requests right now after get() and do not queue them? I try sync this using QSemaphore (acquire before get() then release in finishedImg()) but without success. Or may be paintEvent is not good place for this all? Which one is?

ZapB

Do not start such things from a paintEvent(). You will start fresh requests every time your MainWindow is painted! That will very quickly cripple you system if you get a lot of paintEvents.

QNAM has a limit on how many requests it deals with at once (I think it is 6 iirc). But first things first you need to find somewhere more appropriate to issue your network requests. How would you like to trigger the fetching of the images? Automatically at app start? In response to some user action?

Edit: Ah forget the first part of my response. I didn't read your code properly - sorry. Still a better way might be to simply use your own slot that is called in response to a single shot timer.

ZapB

One way to pace this a little would be to use the receipt of one image to trigger the download of the next one, maybe with a time delay in between.

Or use the producer/consumer pattern where you maintain a buffer of images, a producer component featches the images and appends them to the buffer. Whenthe buffer is full the producer waits (or blocks). A consumer then removes them at a suitable rate and when it is empty the consumer waits (or blocks).

Gourmand

bq. How would you like to trigger the fetching of the images? Automatically at app start?[/quote]

yes, automatically at app start but must show images - that means MainWindow should be properly initialized and able to support this

bq. Still a better way might be to simply use your own slot that is called in response to a single shot timer.

that's an idea... I can start timer in MainWindow::MainWindow()

bq. One way to pace this a little would be to use the receipt of one image to trigger the download of the next one, maybe with a time delay in between.

I can sync images using QSemaphore, but I need to finish each one before next without queuing

bq. Or use the producer/consumer pattern where you maintain a buffer of images, a producer component featches the images and appends them to the buffer.

this looks too complex and wasting resources

ZapB

Why do you think that the producer/consumer pattern is too expensive and wasteful of resources? You yourself have already suggested using a QSemaphore which is used to synchronise access to data from multiple threads (unless you like blocking GUI's of course ;-)).

Using the producer consumer pattern just requires 2 semaphores (or equivalently two wait conditions) as shown in the Qt examples.

Another option that may be of interest to you would be to use a very simple state machine. You would only need a small number of states and transitions.

Gourmand

bq. Why do you think that the producer/consumer pattern is too expensive and wasteful of resources?

I thought you suggest extra image buffering. This is not needed.

Looks like problem appears not because of get() placed in paintEvent(). Images list I get() as well. But then I try get() images from list finishing slot. That can be a source of problem. One QNAM did not properly finish it's task but another one tries to start new task. I have try sync list processing with images.

Gourmand

nope... :-\ get() finishing slot is called somewhere after paintEvent() finished, probably this is because QApplication a.exec() starts later

I'm weird... where better call QNAM instead of paintEvent()? I don't know something like "main thread" in QApplication or it's parent classes.

ZapB

No, in fact your MainWindow::paintEvent() will not get called until the event loop is entered via your call to QApplication::exec(). I suggest that you go read some of the documentation on Qt basics, the event loop, asynchronous operation (for QNAM). I think this should be possible without threading but you may wish to have a quick look at QThread too - especially if the processing of each image is very CPU intensive. Out of interest what are you wanting to do to each image?

To trigger the first fetch without using paintEvent() and to process it and request the next one just do something along these lines:

@
MainWindow::MainWindow( QWidget* parent )
: QMainWindow( parent ),
m_nam( new QNetworkAccessManager( this ) ),
m_currentIndex( -1 ),
m_urlList()
{
// Prepare url list
m_urlList.append( "http://..." );
...

// Kick things off
QTimer::singlShot( 0, SLOT( requestNextImage() ) );

}

MainWindow::requestNextImage()
{
if ( ++m_currentIndex < m_urlList.size() ) {
QNetworkReply* reply = m_nam->get( m_urlList.at( m_currentIndex ) )
connect( reply, SIGNAL( finished() ), this, SLOT( processImage() ) );
// Connect other signal as required
} else {
qDebug() << "All Done!";
}
}

MainWindow::processImage()
{
QNetworkReply* reply = qobject_cast<QNetworkReply*>( sender() );
if ( !reply )
return;

// Process the image as you need
QImage myProcessedImage = ...

// Display processed image
ui->label->setPixmap( myProcessedImage );

// Start fetch of next image in a little while
QTimer::singleShot( delay, SLOT( requestNextImage() ) );

}
@

Gourmand

bq. No, in fact your MainWindow::paintEvent() will not get called until the event loop is entered via your call to QApplication::exec().

I know this all and was greatly surprised with results.

But task looks like SOLVED - not exactly the same way you propose but similar

1st list must be downloaded from server, this starts with QTimer from MainWindow::Mainwindow()

2nd after it is downloaded, in finishing slot I start another QTimer to start slot with get() of 1st picture from list

finally, in the finishing slot for picture, after it's processing, I start another QTimer to start slot with get() of each next picture

this works...

Looks like problem was exactly because I tried get() next picture right from finishing slot of previous get(). This does not work. May be this should be explained in QNAM manual. If only it was, I'd save more than day.

ZapB

It is not a problem with QNAM and so this does not belong in its documentation. The problem is that you misunderstood how QNAM and event-driven applications in general work. You need the event loop to be able to process events for things to be able to happen. In this case "things" means asynchronous network requests and painting of images, and yes even your initial call to MainWindow::paintEvent().

Once you realise the importance of allowing the event loop to spin in between tasks it becomes really quite simple.

Gourmand

I know about event loop in Qt since 1997 and in GUI-systems since OS/2. I made some experiments - problem was exactly with paintEvent(). It did not finish before get() - therefore QNAM did not work as I expected. Of course I don't know how EXACTLY event loop works in Qt and how it relates to QNAM. But I read in manual - QNAM is asynchronous. For me that means it does not interfere to main event loop. But exactly it does. It cannot work properly if event processing was not finished. That MUST be signed in documentation.

ZapB

OK. I wasn't trying to insult you. Apologies if I caused offence. I was trying to say that you had misunderstood how QNAM and sockets and QIODevices in general work in conjunction with the event loop in Qt.

When the above classes are used asynchronously that means that they do require an event loop to be spinning in order for them to work. This line from the docs of QAbstractSocket helps:

"Programming with a blocking socket is radically different from programming with a non-blocking socket. A blocking socket doesn't require an event loop and typically leads to simpler code. However, in a GUI application, blocking sockets should only be used in non-GUI threads, to avoid freezing the user interface. See the network/fortuneclient and network/blockingfortuneclient examples for an overview of both approaches."

It is also mentioned in the docs of QIODevice (which is inherited by QNetworkReply):

"Certain subclasses of QIODevice, such as QTcpSocket and QProcess, are asynchronous. This means that I/O functions such as write() or read() always return immediately, while communication with the device itself may happen when control goes back to the event loop."

If synchronous operation is preferred then using a worker thread is recommended so that you do not block the event loop of the main thread which is needed to paint (amongst other things).

If you still feel strongly that QNAM documentation is lacking then feel free to create a merge request that addresses this via Gitorious. Anyway, I'm glad you resolved the problem.

uani

Bug filed: https://bugreports.qt.io/browse/QTBUG-126991

ChrisW67

Bug filed? Why?

This example:

meets the original brief
starts downloading once the application is displayed
does not do overtly non-painting things in paintEvent() to achieve that
does not require threads

#include "widget.h"
#include <QApplication>
int main(int argc, char *argv[]) {
    QApplication a(argc, argv);
    Widget w;
    w.show();
    return a.exec();
}

#ifndef WIDGET_H
#define WIDGET_H

#include "imageproc.h"

#include <QTextEdit>

class Widget : public QTextEdit
{
    Q_OBJECT

public:
    Widget(QWidget *parent = nullptr);
    ~Widget();

private slots:
    void startWhenDisplayed();
    void listStart(int imageCount);
    void imageProcessed(const QString &summary);
    void listComplete();

private:
    ImageProc *m_imgProc;
};
#endif // WIDGET_H

#include "widget.h"

#include <QTimer>

Widget::Widget(QWidget *parent)
    : QTextEdit(parent)
    , m_imgProc(new ImageProc(this))
{
    resize(640, 480);

    connect(m_imgProc, &ImageProc::listStart,      this, &Widget::listStart);
    connect(m_imgProc, &ImageProc::imageProcessed, this, &Widget::imageProcessed);
    connect(m_imgProc, &ImageProc::listComplete,   this, &Widget::listComplete);

    // Eliminates the rubbish with the paintEvent()
    QTimer::singleShot(0, this, &Widget::startWhenDisplayed);
}

Widget::~Widget() {}

void Widget::startWhenDisplayed() {
    setText("Starting image list fetch...\n");
    m_imgProc->fetchImageList(QUrl("http://localhost:8000/list.txt"));
}

void Widget::listStart(int imageCount) {
    append(QString("  Received list of %1 urls\n").arg(imageCount));
}

void Widget::imageProcessed(const QString &summary) {
    append(QString("  Received and processed image: %1\n").arg(summary));
}

void Widget::listComplete() {
    append("... completed.");
}

#ifndef IMAGEPROC_H
#define IMAGEPROC_H

#include <QObject>
#include <QNetworkAccessManager>
#include <QNetworkReply>
#include <QUrl>
#include <QImage>

class ImageProc : public QObject {
    Q_OBJECT

public:
    explicit ImageProc(QObject *parent = nullptr);
    void fetchImageList(const QUrl &url);

signals:
    void listStart(int imageCount);
    void imageProcessed(const QString &summary);
    void listComplete();

private slots:
    void replyReceived(QNetworkReply *reply);

private:
    void fetchNextImage();
    void processImage(QImage image);

private:
    QNetworkAccessManager *m_nam;
    bool m_haveList;
    QList<QUrl> m_urlList;
};

#endif // IMAGEPROC_H

#include "imageproc.h"


ImageProc::ImageProc(QObject *parent)
    : QObject(parent)
    , m_nam(new QNetworkAccessManager(this))
    , m_haveList(false)
    , m_urlList()
{
    connect(m_nam, &QNetworkAccessManager::finished, this, &ImageProc::replyReceived);
    // connect error handler etc...
}

void ImageProc::fetchImageList(const QUrl &url)
{
    QNetworkReply *reply = m_nam->get(QNetworkRequest(url));
    Q_UNUSED(reply);
}

void ImageProc::replyReceived(QNetworkReply *reply)
{
    if (reply->error() == QNetworkReply::NoError) {
        if (m_haveList) {
            // should be receiving an image
            QByteArray data = reply->readAll();
            QImage image;
            image.loadFromData(data);
            bool ok = image.loadFromData(data);
            if (ok)
                processImage(image);
        }
        else {
            // receiving a list
            QTextStream stream(reply);
            QString url;
            while (stream.readLineInto(&url)) {
                m_urlList.append(QUrl(url));
            }
            m_haveList = true;
            emit listStart(m_urlList.count());
        }

        // either way try to fetch an image
        fetchNextImage();
    }

    reply->deleteLater();
}

void ImageProc::fetchNextImage()
{
    if (m_urlList.isEmpty()) {
        m_haveList = false;
        emit listComplete();
        return;
    }

    QUrl url = m_urlList.takeFirst();
    QNetworkReply *reply = m_nam->get(QNetworkRequest(url));
    Q_UNUSED(reply);
}

void ImageProc::processImage(QImage image)
{
    QString summary = QString("%1x%2 depth %3")
                          .arg(image.width())
                          .arg(image.height())
                          .arg(image.depth());
    emit imageProcessed(summary);
}

QT       += core gui network

greaterThan(QT_MAJOR_VERSION, 4): QT += widgets

CONFIG += c++17

# You can make your code fail to compile if it uses deprecated APIs.
# In order to do so, uncomment the following line.
#DEFINES += QT_DISABLE_DEPRECATED_BEFORE=0x060000    # disables all the APIs deprecated before Qt 6.0.0

SOURCES += \
    main.cpp \
    imageproc.cpp \
    widget.cpp

HEADERS += \
    imageproc.h \
    widget.h

# Default rules for deployment.
qnx: target.path = /tmp/$${TARGET}/bin
else: unix:!android: target.path = /opt/$${TARGET}/bin
!isEmpty(target.path): INSTALLS += target

When served this list and images:

$ cat list.txt 
http://localhost:8000/image1.jpg
http://localhost:8000/image2.jpg
http://localhost:8000/image3.jpg
http://localhost:8000/image4.jpg
http://localhost:8000/image5.jpg

$ identify *.jpg
image1.jpg JPEG 100x100 100x100+0+0 8-bit sRGB 7166B 0.000u 0:00.000
image2.jpg JPEG 200x100 200x100+0+0 8-bit sRGB 12689B 0.000u 0:00.000
image3.jpg JPEG 300x100 300x100+0+0 8-bit sRGB 18501B 0.000u 0:00.000
image4.jpg JPEG 400x100 400x100+0+0 8-bit sRGB 23201B 0.000u 0:00.000
image5.jpg JPEG 500x100 500x100+0+0 8-bit sRGB 28794B 0.000u 0:00.000

Does this:

Would have worked 13 years ago.

uani

Because it does not work for him 13 years ago and it does not work for me last week under the same circumstances and is remedied by the same remedy.
As such there is either a bug or a documentation shortcoming.
In the bug filed I already stated the issue does not occur for the most trivial of cases such as yours thus it is likely no bug in general.
Using https, errors occur which are not delivered to sslErrors, this might be a contributor to the observed behavior, the cause is yet to be identified.

Christian Ehrlicher

As long as you can't provide a minimal compileable example the bug is on your side. Prove me wrong.

uani

well, likely not. not much can be done wrong using QNAM, particularly after having combed through the documentation. I found many StackOverflow posts exist mentioning an unidentified issue. Eg. https://stackoverflow.com/questions/2776640/qt-qnetworkaccessmanager-does-not-emit-signals . Identifying the real culprit from those posts is difficult. One such SO post let surmise QNAM is created in a paintEvent (a globe map service url was used). I managed to find this qt.io post. The issue existed, the issue exists. Something triggers the issue in Qt. (Or it is by design but still something adds to it.) I haven't found it yet. Of course, until you haven't proof -- and even if you have -- it's up to your desire and interest to look into QNAM.

ChrisW67

@uani said in How to synchronize QNetworkAccessManager ?:

Identifying the real culprit from those posts is difficult.

Yes, and this thread is no different. Diagnosing the problem with your code requires understanding the entire problem space and eliminating possible causes from most likely to least likely. If you, the OP in this thread, and the poster in your link will not engage with that process then you are going to have to live with the result.

uani

No. QNAM and slot connections have no multiple paths in the documentation.

Don‘t dare to write about my engagement.

Everything relevant has been stated.

Only reply if your to be reply contributes to a possible solution.

artwaw

I can't say I appreciate your attitude.
having said that - it would not hurt if you read with a modicum of understanding. If more experienced developers tell you that your design pattern is substandard then perhaps take it at the face value?

I used to deal with a use case where I had to design and write client-facing program able to manipulate pdf documents over the WebDAV protocol, enabling users to download and display/print hundreds of small, 1-2 paged files at the time.
I did it all using QNAM since it is more than sufficient for the task.
Thinks to consider:

QNAM has a limit for simultaneous tasks being processed, which - as stated - is six.
That means exactly what it says. You can queue/fire 500 downloads in one series but they will be processed in batches of six.
As QNAM is asynchronous you can't expect downloads to arrive in order they were called. Obviously, that depends on many factors but my experience just confirms that.
Idea of calling QNAM from the paintEvent is pure bonkers. That's not what it was designed to do in the event chain event.
If you don't have any other trigger (system event, any other async signal/slot to rely on) at least put the fetch loop on timer slot with a second or so delay. Timer can be started from the MainWindow constructor as the last entry, after QNAM and all the connections to it have been done. This is to allow main loop to spin up properly.
if you need to cross-reference requested URIs with the downloaded data (for the purpose of assigning proper file names let's say) you have to keep track of that yourself. The simplest way is QStringList or QStringListModel, where you can keep called URIs and then remove them once the finished() signal passes you reply object. You do have to remember to deleteLater() that reply. Also, in case of a huge number of files memory issues might arise (I remember stress testing my program with 500.000 files in one go, that's where disk cache for the model would come in handy - but YMMV).