Allow http video streaming while downloading #12542

jagannatharjun · 2020-04-18T20:24:01Z

It basically provides the user with an HTTP URL, which can be played with any video player that allows users to watch movies and seek while a file is downloading. Internally it uses HTTP byte serving.

with this QBittorrent is almost "complete" for me 😊

jagannatharjun · 2020-04-18T20:57:49Z

"core" part is complete i.e streaming works, but require some work to be "mergeable", actually I tried to implement this for fun, but to my surprise actually made it work and it was far easier then I thought XD, but I do realize real work starts now.

about the "interface"
Currently, I made some spaghetti to allow async processing in Connection(base\http\connection.h) and IRequestHandler(base\http\IRequestHandler.h), which I think will not be acceptable. New plan

basically IRequestHandler will become an "Empty" interface, and Connection class will use dynamic_cast to determine whether the class is SyncHandler or ASyncHandler and process accordingly

@glassez @Chocobo1 opinions?

PS: I am very new to UML and stuff.

glassez · 2020-04-19T04:33:08Z

IMO, it is another step on the way to make qBittorrent "bloatware".
Not to mention its implementation (it looks like there are a lot of cross-dependencies again, etc.)... but I will only review it if this "feature" is approved generally.

Chocobo1 · 2020-04-19T06:10:43Z

While it is a cool feature, unfortunately it doesn't really align with this project vision. It should be a good idea to maintain such a fork (on your side) and users could have more choices.

jagannatharjun · 2020-04-19T19:36:18Z

IMO, it is another step on the way to make qBittorrent "bloatware".

, unfortunately it doesn't really align with this project vision.

Since QBittorrent isn't good with handling large number of torrents and "real" seeders will use a barebone libtorrent wrappers anyway. I think our target audience consists of casual torrenters. Utorrent and many other clients have this feature and at the end of the day, it's pretty useful for people who just use torrent for movies which is the majority. I personally use this a lot and had to use Web torrent until now and then shift my torrents to qBittorrent for seeding (don't want to use Utorrent). It would be quite useful for me if you guys let me get this in. I can help with maintenance.

It should be a good idea to maintain such a fork (on your side)

I do this for fun and learning and maintaining a fork would be too much for me. Personally I think it would not be successful anyway, not many people run anything from the internet, I would never.

I hope you guys will see it more from the user side.

sledgehammer999 · 2020-04-19T22:21:18Z

@jagannatharjun I need some clarification since the title is minimal an now description is given in the opening comment. And I am not going to look through all that code.

What does this do? Does it stream local video files to someone connected to the qbt-machine via http?

FranciscoPombal · 2020-04-19T22:41:50Z

@jagannatharjun I think qbittorrent-nox has the potential of competing with the more barebones libtorrent wrappers. The WebAPI and WebUI just need a little more work.

Anyway, I must say the purpose of this PR is also not entirely clear to me. What does this accomplish that is different from simply downloading sequentially and opening the file? I will build this and test it during the week to have a closer look, but I'd also appreciate a more detailed explanation.

Thanks for the initiative anyway.

Chocobo1 · 2020-04-20T05:30:51Z

Since QBittorrent isn't good with handling large number of torrents and "real" seeders will use a barebone libtorrent wrappers anyway.

Is it affecting GUI or WebUI? I think supporting more workload is always the goal for qbittorrent ;)

Does it stream local video files to someone connected to the qbt-machine via http?

AFAIK it gives the user a URL for a movie file on server, and the user can open it with a media player and play it (and probably seek it).

jagannatharjun · 2020-04-20T07:25:12Z

Is it affecting GUI or WebUI? I think this is always the goal for qbittorrent ;)

I have some "seeders" friends, they tend to use rtorrent or deluge for seedboxes. They imply that qbittorrent is too "heavy" for seedboxes and can't handle 100+ torrent. But that's a different debate. And I still stand by that our main user base is "normal" people, who just download torrents to watch movies and this feature would be very useful for this group of people.

What does this do? Does it stream local video files to someone connected to the qbt-machine via http?

Sorry about that. this lockdown is getting on my head🤦‍♂️
about the feature, as @Chocobo1 said it gives the user an URL to play video files while downloading. The main highlight is seeking and other related user experience improvements. This simply uses Http byte serving for implementation, which is very simple to implement and shouldn't pose any maintenance burden,

FranciscoPombal · 2020-04-20T10:43:47Z

I have some "seeders" friends, they tend to use rtorrent or deluge for seedboxes. They imply that qbittorrent is too "heavy" for seedboxes and can't handle 100+ torrent. But that's a different debate. And I still stand by that our main user base is "normal" people, who just download torrents to watch movies.

People using qbittorrent in a headless setting got the impression that it does not handle 100+ torrents well because they are most likely using the webUI, and for a long time the webUI was badly misusing the webAPI in a way that severely impacted performance. This was relatively recently fixed in #11635. Before that, just clicking around the WebUI would send more and more requests before the current one was complete, essentially DOS'ing the instance. Now, even on slow machines the experience is good with up to ~1000 torrents. After that, the steady performance degradation starts to become noticeable. A faster machine may still handle several thousands very well, but qBittorrent is definitely not capable yet of handling several thousands or 10k+ torrents in general with the current webAPI implementation. But the "10k" problem is another matter entirely.

about the feature, as @Chocobo1 said it gives the user an URL to play video files while downloading. The main highlight is seeking and other related user experience improvements. This simply uses Http byte serving for implementation, which is very simple to implement and shouldn't pose any maintenance burden,

👍

sledgehammer999 · 2020-04-20T11:23:08Z

about the feature, as @Chocobo1 said it gives the user an URL to play video files while downloading.

In that case I have to agree with @glassez and @Chocobo1. This is out of scope for a bittorrent client. Or at least for qBittorrent.
If a user runs a server, then he surely has other means of accessing the storage (eg ssh/ftp).

It would be another story, if this was an optimization to serially download torrent pieces to facilitate "streaming" by simultaneously downloading a torrent and watching it.

I am sorry that you did all this work, but we can't accept it.

jagannatharjun · 2020-04-20T11:28:46Z

I respect your opinion, but most of the users don't run on servers but locally, not all features are useful for everyone but if it is useful for most of them, it should be considered.

If I'm correct Utorrent has this since like 2011

sledgehammer999 · 2020-04-20T11:36:10Z

I respect your opinion, but most of the users don't run on servers but locally

But if they run it locally then why can't simply access the storage themselves?

jagannatharjun · 2020-04-20T11:37:25Z

But if they run it locally then why can't simply access the storage themselves?

The main idea here is to provide seeking.

sledgehammer999 · 2020-04-20T11:45:14Z

The main idea here is to provide seeking.

Can you expand on that? Do you mean seeking a file from a multifile torrent or seeking inside a file?

jagannatharjun · 2020-04-20T11:53:52Z

Can you expand on that? Do you mean seeking a file from a multifile torrent or seeking inside a file?

Seeking in video files while playing, This enables QBittorrent to download that part of the file which is currently playing in the video player.

sledgehammer999 · 2020-04-20T12:02:14Z

Seeking in video files while playing, This enables QBittorrent to download that part of the file which is currently playing in the video player.

So somewhere in your code you implement this:

It would be another story, if this was an optimization to serially download torrent pieces to facilitate "streaming" by simultaneously downloading a torrent and watching it.

Right?
If so, what happens in the above case if the user simply opens said file in their media player? Won't they be able to play it? Only that they won't be able to seek right?

jagannatharjun · 2020-04-20T12:12:15Z

So somewhere in your code you implement this:

In Http byte streaming, one can ask for the part of a file from the server through simple Http request, this is how this PR facilitates the streaming of file, The media player (client) provides us the range, and we provide the file back in the specified range by the following process :

map the range into the piece using torrent_info::map_file
if we don't have that piece, set the deadline_time of that piece so as to prioritize this specific piece and check back after that deadline,
if still don't have that piece repeat until we have that piece
send read_piece request to libtorrent
send back provided data from libtorrent through the http connection to the client (media player) who made the request

basically this PR adds the ability to selectively and promptly download the currently needed chunk of the file by the media player.

If so, what happens in the above case if the user simply opens said file in their media player? Won't they be able to play it? Only that they won't be able to seek right?

yes, they won't be able to seek if they were to directly open from disk, they should open it from the provided URL.

jagannatharjun · 2020-04-20T12:20:56Z

Whenever a user requests to "stream a file", we provide an Http URL, if that URL is opened with media player, we follow http byte serving protocol and serve the file in this manner

map the range into the piece using torrent_info::map_file

if we don't have that piece, set the deadline_time of that piece so as to prioritize this specific piece and check back after that deadline,
if still don't have that piece repeat until we have that piece

send read_piece request to libtorrent

send back provided data from libtorrent through the http connection to the client (media player) who made the request

FranciscoPombal · 2020-04-20T13:05:13Z

@jagannatharjun thanks for the clarifications. Are the seeked pieces still saved on disk, same as any other piece? If so, this seems like it could be a straight improvement over simply downloading sequentially - the files get saved all the same, but with this method the user has the freedom to seek around while playing, outside of strict sequential order.

jagannatharjun · 2020-04-20T13:10:17Z

Are the seeked pieces still saved on disk, same as any other piece?

yes, they are still saved, there is no difference from that end.

sledgehammer999 · 2020-04-20T18:25:42Z

After the explanation/clarification given in the previous comments I can see that this feature might be useful to some users. Now I am not entirely against it.

Does anyone else feel like this is ok to add?

jagannatharjun · 2020-04-28T15:39:07Z

Current Implementation works but doesn't follow RFC fully. Had to reimplement a large part of it and I don't think I can use Http::Server and Http::Connection as they are and refactoring them is just too much so have to provide separate implementation but I don't that will add too much code. A lot of "logic" based code can be just reused from Http::* like RequestParser etc.

@glassez @Chocobo1 any chance of getting this in or should I close this PR 😊?

glassez · 2020-04-28T16:49:29Z

I don't think I can use Http::Server and Http::Connection as they are and refactoring them is just too much

It's a generally bad idea to make something universal if it ends up being equally bad for everyone. If it can't be equally good, it's better to have different implementations for different tasks.
You can try to extract some lower level things and share them if possible (according to the sentence above).

jagannatharjun · 2020-04-28T16:59:00Z

It's a generally bad idea to make something universal if it ends up being equally bad for everyone. If it can't be equally good, it's better to have different implementations for different tasks.
You can try to extract some lower level things and share them if possible (according to the sentence above).

The problem is that all clients actually request whole file in a single request, so then I have to store the connection for a very long time send response in chunks, this is bit difficult with the current setup of Http::Server and Http::Connection. moreover, I have to do this in asynchronous manner

Also with this response, I assume you approve this feature 😊, will continue working on it.

glassez · 2020-04-28T17:05:31Z

Also with this response, I assume you approve this feature

...if it "looks nice" in the end (not as something "alien", breaking code design and unmaintainable).

jagannatharjun · 2020-11-16T13:12:14Z

with the latest push, This PR is complete specification wise and "works". So if anyone wants to try out they can do it by downloading files from Github action or compiling themselves. PS: I may not able to complete it for review till next year.

glassez

I tried to start dealing with this PR, but there are so many Coding Style violations here that I'm starting to get bogged down in it... Could you get it ready for review first?

glassez · 2020-12-25T19:26:50Z

src/base/streaming/streamfile.cpp

+    , m_torrentHandle(torrentHandle)
+{
+    const BitTorrent::TorrentInfo info = m_torrentHandle->info();
+    m_name = info.name() + '/' + info.fileName(fileIndex);


Why not m_name = info.fileName(fileIndex);? Or even m_name = torrentHandle->fileName(fileIndex);

glassez · 2020-12-26T06:02:00Z

src/gui/search/searchwidget.cpp

@@ -328,6 +328,7 @@ void SearchWidget::on_searchButton_clicked()
    QString tabName = pattern;
    tabName.replace(QRegularExpression("&{1}"), "&&");
    m_ui->tabWidget->addTab(newTab, tabName);
+    //m_ui->tabWidget->setTabsClosable(true);


Seems like unrelated changes...

glassez · 2020-12-26T06:02:16Z

src/gui/progressbardelegate.cpp

@@ -64,7 +64,8 @@ void ProgressBarDelegate::paint(QPainter *painter, const QStyleOptionViewItem &o

    QStyleOptionProgressBar newopt;
    newopt.initFrom(&m_dummyProgressBar);
-    newopt.rect = option.rect;
+    static_cast<QStyleOption &>(newopt) = static_cast<const QStyleOption &>(option);
+    newopt.palette = m_dummyProgressBar.palette();


Seems like unrelated changes...

glassez · 2020-12-26T07:38:56Z

src/base/streaming/streamingserver.h

+    using QTcpServer::QTcpServer;
+
+    void start();
+private:


Please add empty line above.

glassez · 2020-12-26T07:41:10Z

src/base/streaming/streamingserver.h

+#include <QTcpSocket>
+#include <QElapsedTimer>
+
+#include "../indexrange.h"


Please use base/indexrange.h and so on.

glassez · 2020-12-26T07:44:16Z

src/base/streaming/streamingserver.h

+#include "../indexrange.h"
+#include "../http/types.h"
+
+class HttpSocket : public QObject


It's better to put this class into separate files.

glassez · 2020-12-26T07:52:35Z

src/base/streaming/streamingmanager.cpp

+
+StreamFile *StreamingManager::findFile(int fileIndex, BitTorrent::TorrentHandle *torrentHandle) const
+{
+    const auto fileIter =


Wrong coding style.
Please use the following:

const auto fileIter = std::find_if(m_files.begin(), m_files.end() , [fileIndex, torrentHandle](const StreamFile *file) { return file->isSameFile(fileIndex, torrentHandle); });

glassez · 2020-12-26T08:04:59Z

src/base/streaming/streamingmanager.cpp

+                m_files.remove(m_files.indexOf(file));
+        });
+    }
+    QProcess::startDetached(vlcPath(), {url(file)});


Sorry, what is it for?

glassez · 2020-12-26T08:05:50Z

src/base/streaming/streamingmanager.cpp

+        connect(BitTorrent::Session::instance(), &BitTorrent::Session::torrentAboutToBeRemoved, file, [this, torrentHandle, file](BitTorrent::TorrentHandle *handle) 
+        {
+            if (handle == torrentHandle)
+                m_files.remove(m_files.indexOf(file));


Isn't it a memory leak?

glassez · 2020-12-26T08:08:35Z

src/base/bittorrent/torrentinfo.h

@@ -47,6 +47,13 @@ namespace BitTorrent
    class InfoHash;
    class TrackerEntry;

+    struct PieceFileInfo 


What does this structure describe? Index of what? Start of what? Length of what?

jagannatharjun · 2020-12-26T11:33:07Z

I tried to start dealing with this PR, but there are so many Coding Style violations here that I'm starting to get bogged down in it... Could you get it ready for review first?

Thanks for taking a look, but this is not ready for review, I and my friends are actually using this in our builds for the last couple of months and I know of a couple of edge cases I have to deal with. I actually submit this work as my minor project this semester :), I will probably open a new PR when it's complete. I should be able to complete this by mid-January. Until then I'm leaving this open just in case if anyone wants to try it out.

github-actions · 2021-08-12T00:46:38Z

This PR is stale because it has been 60 days with no activity. This PR will be automatically closed within 7 days if there is no further activity.

github-actions · 2021-08-20T00:46:13Z

This PR was closed because it has been stalled for some time with no activity.

jagannatharjun marked this pull request as draft April 18, 2020 20:25

glassez requested a review from Chocobo1 April 19, 2020 06:03

jagannatharjun changed the title ~~Allow http video streaming~~ Allow http video streaming while downloading Apr 20, 2020

jagannatharjun force-pushed the stream branch from 13038cc to 539a288 Compare November 16, 2020 13:03

base

7f9306c

jagannatharjun added 13 commits November 16, 2020 18:36

implement basic server

e52e535

implement serving

7cd1c92

make it work

1e0fbd8

fix

49254de

log

6a55c2e

work

3912fd9

new

ea6b1b5

rebase

6f06772

rebase session

f7a8256

fix progressbar

5ff6799

direct play

153bc4e

feed it on demand

a8fe189

only delete requests object after receiveing notification

24d25fe

jagannatharjun force-pushed the stream branch from 539a288 to 24d25fe Compare November 16, 2020 13:08

glassez reviewed Dec 26, 2020

View reviewed changes

This was referenced Jan 16, 2021

Can qbittorrent play while downloading? How can the downloaded data be sliced into hls and played #14226

Closed

Extend HTTP server to handle more scenarios #14234

Open

jagannatharjun mentioned this pull request Feb 4, 2021

Add advanced options for piece control defaults #14338

Closed

github-actions bot added the Stale label Aug 12, 2021

github-actions bot closed this Aug 20, 2021

Allow http video streaming while downloading #12542

Allow http video streaming while downloading #12542

Conversation

jagannatharjun commented Apr 18, 2020 • edited Loading

jagannatharjun commented Apr 18, 2020 • edited Loading

glassez commented Apr 19, 2020

Chocobo1 commented Apr 19, 2020 • edited Loading

jagannatharjun commented Apr 19, 2020

sledgehammer999 commented Apr 19, 2020

FranciscoPombal commented Apr 19, 2020

Chocobo1 commented Apr 20, 2020 • edited Loading

jagannatharjun commented Apr 20, 2020 • edited Loading

FranciscoPombal commented Apr 20, 2020

sledgehammer999 commented Apr 20, 2020

jagannatharjun commented Apr 20, 2020 • edited Loading

sledgehammer999 commented Apr 20, 2020

jagannatharjun commented Apr 20, 2020

sledgehammer999 commented Apr 20, 2020

jagannatharjun commented Apr 20, 2020

sledgehammer999 commented Apr 20, 2020

jagannatharjun commented Apr 20, 2020 • edited Loading

jagannatharjun commented Apr 20, 2020

FranciscoPombal commented Apr 20, 2020

jagannatharjun commented Apr 20, 2020

sledgehammer999 commented Apr 20, 2020

jagannatharjun commented Apr 28, 2020

glassez commented Apr 28, 2020

jagannatharjun commented Apr 28, 2020 • edited Loading

glassez commented Apr 28, 2020

jagannatharjun commented Nov 16, 2020

glassez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jagannatharjun commented Dec 26, 2020

github-actions bot commented Aug 12, 2021

github-actions bot commented Aug 20, 2021

jagannatharjun commented Apr 18, 2020 •

edited

Loading

jagannatharjun commented Apr 18, 2020 •

edited

Loading

Chocobo1 commented Apr 19, 2020 •

edited

Loading

Chocobo1 commented Apr 20, 2020 •

edited

Loading

jagannatharjun commented Apr 20, 2020 •

edited

Loading

jagannatharjun commented Apr 20, 2020 •

edited

Loading

jagannatharjun commented Apr 20, 2020 •

edited

Loading

jagannatharjun commented Apr 28, 2020 •

edited

Loading