Improve sequentialDownload/firstLastPiecePriority handling

Always store firstLastPiecePriority in resume data.
Don't recalculate firstLastPiecePriority value each time it is accessed.
Always store sequentialDownload in resume data as native libtorrent field.

src/base/bittorrent/session.cpp

@@ -1974,7 +1974,6 @@ LoadTorrentParams Session::initLoadTorrentParams(const AddTorrentParams &addTorr
 
     loadTorrentParams.name = addTorrentParams.name;
     loadTorrentParams.tags = addTorrentParams.tags;
-    loadTorrentParams.sequential = addTorrentParams.sequential;
     loadTorrentParams.firstLastPiecePriority = addTorrentParams.firstLastPiecePriority;
     loadTorrentParams.hasSeedStatus = addTorrentParams.skipChecking; // do not react on 'torrent_finished_alert' when skipping
     loadTorrentParams.hasRootFolder = ((addTorrentParams.createSubfolder == TriStateBool::Undefined)
@@ -2079,6 +2078,11 @@ bool Session::addTorrent_impl(const AddTorrentParams &addTorrentParams, const Ma
     // Preallocation mode
     p.storage_mode = isPreallocationEnabled() ? lt::storage_mode_allocate : lt::storage_mode_sparse;
 
+    if (addTorrentParams.sequential)
+        p.flags |= lt::torrent_flags::sequential_download;
+    else
+        p.flags &= ~lt::torrent_flags::sequential_download;
+
     // Seeding mode
     // Skip checking and directly start seeding
     if (addTorrentParams.skipChecking)
@@ -3983,7 +3987,6 @@ bool Session::loadTorrentResumeData(const QByteArray &data, const TorrentInfo &m
     torrentParams.name = fromLTString(root.dict_find_string_value("qBt-name"));
     torrentParams.savePath = Profile::instance()->fromPortablePath(
         Utils::Fs::toUniformPath(fromLTString(root.dict_find_string_value("qBt-savePath"))));
-    torrentParams.sequential = root.dict_find_int_value("qBt-sequential");
     torrentParams.hasSeedStatus = root.dict_find_int_value("qBt-seedStatus");
     torrentParams.firstLastPiecePriority = root.dict_find_int_value("qBt-firstLastPiecePriority");
     torrentParams.hasRootFolder = root.dict_find_int_value("qBt-hasRootFolder");
@@ -4043,9 +4046,16 @@ bool Session::loadTorrentResumeData(const QByteArray &data, const TorrentInfo &m
             if (addedTimeNode.type() == lt::bdecode_node::int_t)
                 p.added_time = addedTimeNode.int_value();
 
+            const lt::bdecode_node sequentialNode = root.dict_find("qBt-sequential");
+            if (sequentialNode.type() == lt::bdecode_node::int_t) {
+                if (static_cast<bool>(sequentialNode.int_value()))
+                    p.flags |= lt::torrent_flags::sequential_download;
+                else
+                    p.flags &= ~lt::torrent_flags::sequential_download;
+            }
+
             if (torrentParams.name.isEmpty() && !p.name.empty())
                 torrentParams.name = QString::fromStdString(p.name);
-
         }
         // === END DEPRECATED CODE === //
         else {

src/base/bittorrent/torrenthandleimpl.cpp

@@ -126,6 +126,7 @@ TorrentHandleImpl::TorrentHandleImpl(Session *session, const lt::torrent_handle
     , m_hasSeedStatus(params.hasSeedStatus)
     , m_hasRootFolder(params.hasRootFolder)
     , m_useAutoTMM(params.savePath.isEmpty())
+    , m_hasFirstLastPiecePriority(params.firstLastPiecePriority)
     , m_ltAddTorrentParams(params.ltAddTorrentParams)
 {
     if (m_useAutoTMM)
@@ -134,22 +135,13 @@ TorrentHandleImpl::TorrentHandleImpl(Session *session, const lt::torrent_handle
     updateStatus();
     m_hash = InfoHash(m_nativeStatus.info_hash);
 
-    // NB: the following two if statements are present because we don't want
-    // to set either sequential download or first/last piece priority to false
-    // if their respective flags in data are false when a torrent is being
-    // resumed. This is because, in that circumstance, this constructor is
-    // called with those flags set to false, even if the torrent was set to
-    // download sequentially or have first/last piece priority enabled when
-    // its resume data was saved. These two settings are restored later. But
-    // if we set them to false now, both will erroneously not be restored.
-    if (!params.restored || params.sequential)
-        setSequentialDownload(params.sequential);
-    if (!params.restored || params.firstLastPiecePriority)
-        setFirstLastPiecePriority(params.firstLastPiecePriority);
-
-    if (!params.restored && hasMetadata()) {
-        if (filesCount() == 1)
-            m_hasRootFolder = false;
+    if (hasMetadata()) {
+        applyFirstLastPiecePriority(m_hasFirstLastPiecePriority);
+
+        if (!params.restored) {
+            if (filesCount() == 1)
+                m_hasRootFolder = false;
+        }
     }
 
     // TODO: Remove the following upgrade code in v.4.4
@@ -749,21 +741,7 @@ bool TorrentHandleImpl::isSequentialDownload() const
 
 bool TorrentHandleImpl::hasFirstLastPiecePriority() const
 {
-    if (!hasMetadata())
-        return m_needsToSetFirstLastPiecePriority;
-
-    const std::vector<lt::download_priority_t> filePriorities = nativeHandle().get_file_priorities();
-    for (int i = 0; i < static_cast<int>(filePriorities.size()); ++i) {
-        if (filePriorities[i] <= lt::download_priority_t {0})
-            continue;
-
-        const TorrentInfo::PieceRange extremities = info().filePieces(i);
-        const lt::download_priority_t firstPiecePrio = nativeHandle().piece_priority(lt::piece_index_t {extremities.first()});
-        const lt::download_priority_t lastPiecePrio = nativeHandle().piece_priority(lt::piece_index_t {extremities.last()});
-        return ((firstPiecePrio == lt::download_priority_t {7}) && (lastPiecePrio == lt::download_priority_t {7}));
-    }
-
-    return false;
+    return m_hasFirstLastPiecePriority;
 }
 
 TorrentState TorrentHandleImpl::state() const
@@ -1245,17 +1223,24 @@ void TorrentHandleImpl::setSequentialDownload(const bool enable)
 
 void TorrentHandleImpl::setFirstLastPiecePriority(const bool enabled)
 {
-    setFirstLastPiecePriorityImpl(enabled);
+    if (m_hasFirstLastPiecePriority == enabled)
+        return;
+
+    m_hasFirstLastPiecePriority = enabled;
+    if (hasMetadata())
+        applyFirstLastPiecePriority(enabled);
+
+    LogMsg(tr("Download first and last piece first: %1, torrent: '%2'")
+        .arg((enabled ? tr("On") : tr("Off")), name()));
+
+    saveResumeData();
 }
 
-void TorrentHandleImpl::setFirstLastPiecePriorityImpl(const bool enabled, const QVector<DownloadPriority> &updatedFilePrio)
+void TorrentHandleImpl::applyFirstLastPiecePriority(const bool enabled, const QVector<DownloadPriority> &updatedFilePrio)
 {
-    // Download first and last pieces first for every file in the torrent
+    Q_ASSERT(hasMetadata());
 
-    if (!hasMetadata()) {
-        m_needsToSetFirstLastPiecePriority = enabled;
-        return;
-    }
+    // Download first and last pieces first for every file in the torrent
 
     const std::vector<lt::download_priority_t> filePriorities = !updatedFilePrio.isEmpty() ? toLTDownloadPriorities(updatedFilePrio)
                                                                            : nativeHandle().get_file_priorities();
@@ -1281,11 +1266,6 @@ void TorrentHandleImpl::setFirstLastPiecePriorityImpl(const bool enabled, const
     }
 
     m_nativeHandle.prioritize_pieces(piecePriorities);
-
-    LogMsg(tr("Download first and last piece first: %1, torrent: '%2'")
-        .arg((enabled ? tr("On") : tr("Off")), name()));
-
-    saveResumeData();
 }
 
 void TorrentHandleImpl::pause()
@@ -1500,10 +1480,9 @@ void TorrentHandleImpl::handleSaveResumeDataAlert(const lt::save_resume_data_ale
     const bool useDummyResumeData = !p;
     if (useDummyResumeData) {
         resumeData["qBt-magnetUri"] = createMagnetURI().toStdString();
-        // Both firstLastPiecePriority and sequential need to be stored in the
+        // sequentialDownload needs to be stored in the
         // resume data if there is no metadata, otherwise they won't be
         // restored if qBittorrent quits before the metadata are retrieved:
-        resumeData["qBt-firstLastPiecePriority"] = hasFirstLastPiecePriority();
         resumeData["qBt-sequential"] = isSequentialDownload();
 
         resumeData["qBt-addedTime"] = addedTime().toSecsSinceEpoch();
@@ -1518,6 +1497,7 @@ void TorrentHandleImpl::handleSaveResumeDataAlert(const lt::save_resume_data_ale
     resumeData["qBt-name"] = m_name.toStdString();
     resumeData["qBt-seedStatus"] = m_hasSeedStatus;
     resumeData["qBt-hasRootFolder"] = m_hasRootFolder;
+    resumeData["qBt-firstLastPiecePriority"] = m_hasFirstLastPiecePriority;
 
     m_session->handleTorrentResumeDataReady(this, resumeDataPtr);
 }
@@ -1649,12 +1629,10 @@ void TorrentHandleImpl::handleMetadataReceivedAlert(const lt::metadata_received_
         m_session->handleTorrentPaused(this);
     }
 
-    // If first/last piece priority was specified when adding this torrent, we can set it
-    // now that we have metadata:
-    if (m_needsToSetFirstLastPiecePriority) {
-        setFirstLastPiecePriority(true);
-        m_needsToSetFirstLastPiecePriority = false;
-    }
+    // If first/last piece priority was specified when adding this torrent,
+    // we should apply it now that we have metadata:
+    if (m_hasFirstLastPiecePriority)
+        applyFirstLastPiecePriority(true);
 }
 
 void TorrentHandleImpl::handlePerformanceAlert(const lt::performance_alert *p) const
@@ -1898,9 +1876,6 @@ void TorrentHandleImpl::prioritizeFiles(const QVector<DownloadPriority> &priorit
     if (!hasMetadata()) return;
     if (priorities.size() != filesCount()) return;
 
-    // Save first/last piece first option state
-    const bool firstLastPieceFirst = hasFirstLastPiecePriority();
-
     // Reset 'm_hasSeedStatus' if needed in order to react again to
     // 'torrent_finished_alert' and eg show tray notifications
     const QVector<qreal> progress = filesProgress();
@@ -1918,8 +1893,8 @@ void TorrentHandleImpl::prioritizeFiles(const QVector<DownloadPriority> &priorit
     m_nativeHandle.prioritize_files(toLTDownloadPriorities(priorities));
 
     // Restore first/last piece first option if necessary
-    if (firstLastPieceFirst)
-        setFirstLastPiecePriorityImpl(true, priorities);
+    if (m_hasFirstLastPiecePriority)
+        applyFirstLastPiecePriority(true, priorities);
 }
 
 QVector<qreal> TorrentHandleImpl::availableFileFractions() const

src/base/bittorrent/torrenthandleimpl.h

@@ -62,7 +62,6 @@ namespace BitTorrent
         QString category;
         QSet<QString> tags;
         QString savePath;
-        bool sequential = false;
         bool firstLastPiecePriority = false;
         bool hasSeedStatus = false;
         bool hasRootFolder = true;
@@ -281,7 +280,7 @@ namespace BitTorrent
         void move_impl(QString path, MoveStorageMode mode);
         void moveStorage(const QString &newPath, MoveStorageMode mode);
         void manageIncompleteFiles();
-        void setFirstLastPiecePriorityImpl(bool enabled, const QVector<DownloadPriority> &updatedFilePrio = {});
+        void applyFirstLastPiecePriority(bool enabled, const QVector<DownloadPriority> &updatedFilePrio = {});
 
         Session *const m_session;
         lt::torrent_handle m_nativeHandle;
@@ -315,7 +314,7 @@ namespace BitTorrent
         bool m_fastresumeDataRejected = false;
         bool m_hasMissingFiles = false;
         bool m_hasRootFolder;
-        bool m_needsToSetFirstLastPiecePriority = false;
+        bool m_hasFirstLastPiecePriority = false;
         bool m_useAutoTMM;
 
         bool m_unchecked = false;