search

ukdtom / SRT2UTF-8.bundle

Plex Agent, that'll convert sidecar subtitle files into UTF-8, if not
126 stars 14 forks source link

Option to copy original file instead of in-place conversion #22

Closed adriantc closed 9 years ago

adriantc commented 9 years ago

Hello,

First of all thanks for this very useful plugin. One issue I am having is that I don't want the in-place file conversion and, for my use case, a much better solution would be a new file. I have made a fork and will change the code to support such an option. Would you be interested in merging it in your code?

Thanks,

ukdtom commented 9 years ago

Sorry, but don't get it? How would you name the copy, in order for it to be picked up by Plex? And for what purpose? Srt2Utf-8 Does replace the origen one, but that's in order to make sure, that Plex is using it, and you can, if you wanna keep the origen one, simply enable the backup function, which is a simple rename of the origen file

adriantc commented 9 years ago

I don't want to rename it and I need the original file (and its name) intact. The way I have solved the detection thing is that I have moved this agent on top of all other agents so it gets triggered before the library gets refreshed.

ukdtom commented 9 years ago

Sorry, but still doesn't make sense to me....If the origin file is left in place, then what filename to give the converted one? And moving the agent to the top, will break it for new files, IMHO

adriantc commented 9 years ago

The use case is as follows. Some other softwares I use don't like the files they also use get changed in any way. That is why the initial implementation is not good for my use case. While it works perfectly once the conversion is finished the other application breaks.

As for your question. I don't know when the agents are triggered (I expected them to get triggered with each library refresh), but after experimenting I have realized they are only triggered when manually refreshing a movie, which is just great for my use case. (it would be perfect if Plex would have done that when doing maintenance or when it would detect changes, but it is a small price to pay for the functionality that Plex has to offer). So right now for me it is working.

As for the filename.... none of my subtitle files have language code in their name... so if the original filename is xxx.srt the generated filed would be xxx.ro.srt (I am from Romania). This has an added advantage... it is easy for me to see that the plugin did its job... when refreshing the subtitles become Romanian from Unknown in Plex.

ukdtom commented 9 years ago

So in essens....When during a run you let srt2utf-8 do it's job, and when saving, it add's the language code to it, and keep the origen one? What happens, if the new file already has the language code, and when said.....I added a prefs option to assume a language, if there was no language code in the filename, as a req. to a lot of users up there, where their players doesn't accept a language code....

As such, I think, if I understand what you are saying correct, that merging your changes will break it for others?

adriantc commented 9 years ago

Yes. 99.99% of my srt files don't have the language in the name (or at least not in the that format). It is true that if the file already has the language in the name it would end up adding another one. But that can be checked in the code. My question is: Do you consider this an useful option?... If so I can tweak it to cover all scenarios.

ukdtom commented 9 years ago

I fail to see this going to a broader audience, since it's such a special case, but regardless.....If you can make it, so it's selectable in the prefs, and disabled as default, then I see no reason why I shouldn't merge it in....After all, having me jurge peoples needs as the sole dude doesn't seem fair ;-)

ukdtom commented 9 years ago

When making a pull request, please do it against the develop branch

adriantc commented 9 years ago

The thing is I already forked your project. I know it can easily be merged into the master project, but maybe you want to review the changes before that... https://github.com/adriantc/SRT2UTF-8.bundle

I have updated the preferences so the users can understand what the feature does. When the file already exists it will overwrite it for now. As you said... this feature it probably not for the mainstream audience.

adriantc commented 9 years ago

Oh one more thing... Moving the agent on top may break the OpenSubtitles conversion feature, but it also solves the problem of the refresh. Now, even with the copy option on, there is no need to do an extra refresh. If you add something new to the library it triggers the conversion agent before the scanner so the change it realtime.

ukdtom commented 9 years ago

There has never been an issue with refresh, AFAIK?

When @ the bottom, it'll work when new items are added, and only when activating for the first time, a forced refresh is needed

adriantc commented 9 years ago

True... in your implementation the refresh was not needed because the scanner already seen the file. And since it was an in-place change it didn't matter for Plex. The file was already indexed.

However if it is copied (like the way I need it)... the order of the agents becomes important. If it isn't before the scanner agent then the file is generated only after the movie and the original subtitles are indexed. As such it won't detect it until a new refresh. If it is first in the order... the new file is generate and then the media scanner indexes it.

The order of the agents is not important if modifying the original file. If copying it then there are 2 solutions: a. move conversion agent before the scanner agent (the OpenSub functionality may be impacted) b. do an extra refresh so the new file an be indexed.