cfpp2p / transmission

Transmission bittorrent with p2p streaming. Fast, stable, feature rich - see Wiki
https://github.com/cfpp2p/transmission/wiki/Where-to-get-general-support
Other
43 stars 13 forks source link

[Request] Adding group #1

Closed Belphemur closed 9 years ago

Belphemur commented 9 years ago

Hello @cfpp2p,

I checked on the tracker of transmission and found a full patch for adding group support: https://trac.transmissionbt.com/ticket/5385

I took the liberty to update it for r14554. I know your fork is based on 2.77, and I have unfortunately no idea how to port the patch without the use of quark.

You can find the patch here: https://gist.github.com/Belphemur/721a73200764388f0fc5

I'm currently using transmission with this patch and the grouping work flawlessly. If you could port it to your fork, I think it would be a great addition.

Cheers

cfpp2p commented 9 years ago

Thank you for the request.

If I'm reading the patch correctly the group ability is placement of the download in a sub-directory of the same named group.

Grouping is already implemented in the GUI. I just made a post here http://sourceforge.net/p/transmissiondaemon/discussion/general/thread/856eb8c9/#49ee to document that grouping feature. The GUI is available for all platforms. The GUI can be local or remote.

My understanding of the patch is that a main focus is the web client. Is that right?

With the web client there currently is no advanced sorting or filtering of the groups. You can at a basic level access groups in the web client for torrents.

It wouldn't be difficult to convert from tr_variant quark to tr_benc char however I don't think all of the patch's changes to libtransmission should be necessary. To enable advanced group features for the web client I would rather use a similar method as the GUI. If I get time, maybe some web client commits implementing similar to GUI. I generally don't like a lot of changes to libtransmission if not needed.

Please let me know if you think I'm correct in my evaluations.Thank you for the request.

Belphemur commented 9 years ago

My usage of this patch is mostly related to the RPC in fact.

I'm using a node.js application to scrap automatically torrent for my shows. When adding the torrent to transmission I set the wanted group.

When a torrent is downloaded, I'm running a python script that will launch FileBot with a set of parameter directly linked to the group in which the torrent belong. This is possible because the patch add the TR_TORRENT_GROUP environment variable linked directly to the assigned group. (If no group assigned when adding the torrent, a default group is configurable).

I share the torrent server with other users for whom FileBot is not needed. I'm filter these user by simply checking if the torrent have the default group assigned.

cfpp2p commented 9 years ago

I could add a simple attribute to every torrent tor->owner that default would be NULL and could be set via RPC

Then when torrentCallScript() is called this would set environment variable TR_TORRENT_OWNER tr_strdup_printf( "TR_TORRENT_OWNER=%s", tor->owner ),

static void
torrentCallScript( const tr_torrent * tor, const char * script )
{
    char timeStr[128];
    const time_t now = tr_time( );

    tr_strlcpy( timeStr, ctime( &now ), sizeof( timeStr ) );
    *strchr( timeStr,'\n' ) = '\0';

    if( script && *script )
    {
        int i;
        char * cmd[] = { tr_strdup( script ), NULL };
        char * env[] = {
            tr_strdup_printf( "TR_APP_VERSION=%s", SHORT_VERSION_STRING ),
            tr_strdup_printf( "TR_TIME_LOCALTIME=%s", timeStr ),
            tr_strdup_printf( "TR_TORRENT_DIR=%s", tor->currentDir ),
            tr_strdup_printf( "TR_TORRENT_ID=%d", tr_torrentId( tor ) ),
            tr_strdup_printf( "TR_TORRENT_HASH=%s", tor->info.hashString ),
            tr_strdup_printf( "TR_TORRENT_NAME=%s", tr_torrentName( tor ) ),
            tr_strdup_printf( "TR_TORRENT_OWNER=%s", tor->owner ),
            NULL };

        tr_torinf( tor, "Calling script \"%s\"", script );

Already implemented is run script on torrent added and done.

An even simpler method would be to just use the existing tor->downloadDir tor->downloadDir is the final set location of a completed download and is different than tor->currentDir The GUI essentially uses tor->downloadDir, strips off everything before the final sub-directory to display the group.

script on torrent added could process based on new environment variable TR_TORRENT_DOWNDIR and change the download directory or not and script on torrent done as you have it.

    "script-torrent-added-enabled": true, 
    "script-torrent-added-filename": "/torrent-added-script", 

Then the only change would be adding this single line

 tr_strdup_printf( "TR_TORRENT_DOWNDIR=%s", tor->downloadDir ),

to torrentCallScript()

What would best satisfy your usage needs?

Belphemur commented 9 years ago

I find redundant to have to use the script-torrent-added and script-torrent-done than just script-torrent-done for the same thing.

A field owner would do the trick for me. I'm aware on how Transmission Remote GUI work for the grouping/auto-grouping, I'm using it already and your modification of it is really nice. (Would be nice to add those group through an interface).

I admit I prefer the principle of group than owner, I'm downloading all the torrent in the same directory (per user) then ask FileBot to do the sorting by creating hardlink in another directory. Depending if it's a Series, a Movie or Anime I want it to act differently. This folder is then synced (using syncthing) on my media center with all the subtitle, image etc ... needed. I don't do any sorting myself.

Previously I was using deluge with its labelPlus plugin that let me set rule for auto-labeling: If name contain/not contain this or that, etc ... Pretty neat. When a torrent was added, those rule were applied and the torrent was automatically set with the right label or sublabel. But because deluge was taking easily 50-60% more resource than transmission I moved back to it and found this patch that +- add what I need.

cfpp2p commented 9 years ago

OK, I'll implement grouping RPC and libtransmission.

I'm downloading all the torrent in the same directory (per user)

What do you mean? One directory for all users or each user get a specific group? I don't know what you mean same directory.

Belphemur commented 9 years ago

Sorry forget the bit about users. The torrent server is using a Php front end with different user. By default each user get its own directory where per default the torrent get created. This is another system in place.

I'm really happy with the grouping.

On 6 August 2015 18:24:03 EEST, cfpp2p notifications@github.com wrote:

OK, I'll implement grouping RPC and libtransmission.

I'm downloading all the torrent in the same directory (per user)

What do you mean? One directory for all users or each user get a specific group? I don't know what you mean same directory.


Reply to this email directly or view it on GitHub: https://github.com/cfpp2p/transmission/issues/1#issuecomment-128409627

Best Regards, Antoine Aflalo

cfpp2p commented 9 years ago

Hi @Belphemur ,

I ported your patch and committed https://github.com/cfpp2p/transmission/commit/2bb09971540d7918572f6485df88aa26ac96188c

web-client-cfp https://github.com/cfpp2p/transmission/tree/master Shift clients tested the group options.

From https://trac.transmissionbt.com/ticket/5385

applying a group to a download will survive restarts (ie. it is saved in resume file)
users can customise their groups through config (transmission-daemon/settings.json)
a default group can be set through config (transmission-daemon/settings.json)
default group can be modified through web interface
in web interface group can be modified through contextmenu
'On Torrent Completion' script is called with extra param 'TR_TORRENT_GROUP' 

web interface displays group by means of coloured block in torrent rows

Everything tested with Shift and functioning correctly.

Environment variables look like this:

TR_APP_VERSION=2.77 TR_TIME_LOCALTIME=Sat Aug 8 00:43:03 2015 TR_TORRENT_DIR=/trs-p-dwnlds TR_TORRENT_GROUP=pop-music TR_TORRENT_HASH=53e403ef686429XXXX503c8f5feaa7d299edac45 TR_TORRENT_ID=5 TR_TORRENT_NAME=True

Let me know how everything works for your system.

There were a couple of issues with your original patch which I fixed all of them in my port. patch line 241

  • if (tr_variantDictFindList (args_in, TR_KEY_download_queue_size, &list))
  • tr_sessionSetDownloadGroups (session, list);

This does nothing, TR_KEY_download_queue_size is int (tr_variantDictFindInt), tr_variantDictFindList will always fail so your line immediately above it is correct and is all that's needed for tr_sessionSetDownloadGroups correctly.

patch line 693

if (key == TR_KEY_download_groups && tr_variantListSize(val) > 0)

compare like this a string literal could lead to undefined behavior

(didn't mean to close this yet, mistakenly wrong button)

Belphemur commented 9 years ago

I couldn't configure the directory because the file po/Makefile.in.in is missing from your repo (or I missed the way to generate it).

I took the one from the official repo, now configure works. Moreover all the bash script miss the executable permission. I just got a build failed with ./updateminiupnpcstrings.sh ./VERSION ./miniupnpcstrings.h.in miniupnpcstrings.h

Other than that it seems to work, I just can't get swift to work. It doesn't populate with the torrents (even if the request to the RPC is done and return a valid result). Tested with Chrome and Firefox. The normal, advanced and experimental web client work.

cfpp2p commented 9 years ago

I took the one from the official repo

Was the file you used the same file as this one ? http://transmissionbt.net/poMakefile-inin.zip

I couldn't configure the directory because the file po/Makefile.in.in is missing from your repo (or I missed the way to generate it).

svn repository building: BuildingfromanSVNsnapshot https://trac.transmissionbt.com/wiki/Building#BuildingfromanSVNsnapshot

re-generate Makefile.in.in : https://answers.launchpad.net/xpad/+question/160246 http://stackoverflow.com/questions/10366852/autogen-sh-does-not-regenerate-po-makefile-in-in http://stackoverflow.com/questions/28122207/how-does-makefile-in-in-get-changed-to-makefile-when-using-intltool

I just got a build failed with ./updateminiupnpcstrings.sh ./VERSION ./miniupnpcstrings.h.in miniupnpcstrings.h

Was that solved when you placed the po/Makefile.in.in ?

make[2]: Entering directory `/c/backup/src/transmission-2.84+/third-party/miniupnp'
./updateminiupnpcstrings.sh ./VERSION ./miniupnpcstrings.h.in miniupnpcstrings.h
Detected OS [Debian] version []
cat: ./VERSION: No such file or directory

I just can't get swift to work. It doesn't populate with the torrents

Like this... give it a few seconds to populate, maybe refresh the browser.

. shift Trackers and Queue . classic

Other than that it seems to work,

By that do you mean it's working as per system setup usage with node.js, your python script, FileBot and the filters for multiple users on the server ?

Belphemur commented 9 years ago
  1. Yes exactly that file. I wasn't aware of the intltoolize command. It should have been runned by the autogen.sh, I think my setup was missing the tool and the script didn't report on it.
  2. Nope it didn't, a good chmod +x **/**/*.sh with another chmod +x *.sh inside the directory. (if my memory is right svn doesn't keep permission where git does)
  3. I did multiple refresh and checked the different request done by the application. No error in the javascript in the Console tab and the RPC request where successfully returning the torrent that should have been displayed. Like said I tried with firefox and chrome (both latest version).

About the setup:

  1. Tranmission client
  2. Torrent-ng (PHP) with multiple user. Each user get their own directory and only see their own torrent. All done by the PHP app, the app give the command using the RPC. I use this app to share the torrent server with a couple of friends. Each of those directory is accessible by ftp and the each user have it's own access. By default the group "Others" is given. When my python script get called, it check the group and in case of "Others", exit directly.
  3. Node-RED with a single user (me). Using Node-RED (Node.js app) I created a series scrapper. It does everyday check for new episode for some shows I'm following directly linked to RARBG. The torrent are added also using RPC with the group "Shows". When the python script is runned and see it's the group "Shows", it run FileBot with some specific argument to force it to see the downloaded torrent as a Show. FileBot create a hardlink into a directory with the correct labeling and directory sorting. (Also use pushbullet to tell me an episode is processed by FileBot and available)
  4. And now Syncthing enter in action, I'm using it to synchronize the folder where FileBot do the hard links with my Media Center at home. Each time a new episode is "sorted", it get synchronized and downloaded by my raspberry pi while the server continue to seed letting me keep my connection without any disruption and still seeding.

With the current patch of 2.84, I'm using also the interface to add torrent as "Movies", same idea than "Shows" just different command for FileBot.

cfpp2p commented 9 years ago

Could you get Shift from here and try it. I'm not sure what the problem is.

https://github.com/killemov/Shift

Belphemur commented 9 years ago

Ho my ... I'm sorry... The Shift client work perfectly. Yours and the original one. My mistake was to think it was by default showing ALL torrents and not only the one downloading and since the status filter wasn't showing by default ...

My bad. This works perfectly fine.

cfpp2p commented 9 years ago

OK, good to hear. But anyway I'm going to add full 2.84+ web client patched with your patch. It looks and functions quite nice. This way all the advanced grouping options are available through the web client too. I've tested with and I'm satisfied.

Thanks for your contributions.

Belphemur commented 9 years ago

That's great news ! The patch was already available, I just updated it for 2.84+, the credit goes to dmitriy and trisooma. All I did is correct a little annoyance in the webclient where it was always resetting the default group in the "Add Torrent" panel.

cfpp2p commented 9 years ago

The normal, advanced and experimental web client work.

and now there's Group Tag View https://github.com/cfpp2p/transmission/archive/master.zip

Belphemur commented 9 years ago

After multiple try and comparison between the 2.84+ and 2.77 with your patch applied. I have one issue.

On the 2.84 patched, the groups are correctly saved in the settings.json. You can set, modify groups in the interface and they get saved.

On the 2.77, the groups never get written in the settings.json. Moreover even if you modify the file manually, transmission is always returning the default groups set in the code.

Any idea why it's doing that ?

cfpp2p commented 9 years ago

I never switched JSON_parser with jsonsl. I think that might cause the reason it happens.

https://trac.transmissionbt.com/changeset/13614 https://trac.transmissionbt.com/ticket/5131#comment:9

If I use shift client to change a torrent's group to something not in the list this new group then shows up in the filter list.

This remains through restarts but I can assign at most three colors no matter how many groups there are, but to whatever groups I want.

Belphemur commented 9 years ago

It seems the patches described here: https://trac.transmissionbt.com/ticket/5131#comment:5 Are compatible also with 2.52, which then should still be compatible with your 2.77 branch.

Moreover Jsonsl seems to still be supported and renowned (and clearly doesn't have the bug mentioned)

For the list it's normal, the filtering of group is based on the filtering of trackers. It checks all torrents and add each one it find into the filter list.

I forked your repo, made the change for Jsonsl: https://github.com/Belphemur/transmission/commit/864f977000f7b9f3ce000b2f9f4657e9ccbe7663

This doesn't change the problem. Transmission still send the default groups instead of the one configured, the problem must be elsewhere.

cfpp2p commented 9 years ago

the problem must be elsewhere.

https://github.com/cfpp2p/transmission/commit/f8524af8e3bd5b1989c4a2b5d3adc196d6e3806b This commit fixed it I believe. Now settings.json automatically saves, like below, and restarts use whatever is in settings.json.

"download-group-default": "movies", 
    "download-groups": [
        [
            "Music", 
            "#ffff00"
        ], 
        [
            "Movies", 
            "#ff0000"
        ], 
        [
            "Software", 
            "#4a86e8"
        ]
    ],  

After multiple try and comparison between the 2.84+ and 2.77 with your patch applied. I have one issue.

I very much appreciate your testing. Could you please verify that the issue(s) you experienced are resolved.

Belphemur commented 9 years ago

I confirm everything works as expected now ! Great Job !

I took the opportunity to make a pull request with jsonsl.