HaveAGitGat / Tdarr_Plugins

Tdarr Plugins
GNU General Public License v3.0
141 stars 164 forks source link

ffmpegCommandRemoveStreamByProperty transodes to incorrect audio/video codecs in "not includes" mode #562

Open andrew-kennedy opened 12 months ago

andrew-kennedy commented 12 months ago

When I transcode any video using the ffmpegCommandRemoveStreamByProperty plugin in "not_includes" mode, it ends up outputting an h264 video with a vorbis audio stream no matter what streams I try to remove. It's very strange.

I was trying to ensure videos I just transcoded to av1 have only an opus audio stream, so I set its property to check to codec_name, the Values to Remove to av1,opus, and the Condition to not_includes mode (implying that I want to remove any streams not in the list), and this ends up always producing a video with h264 video and a single vorbis audio stream, no matter what the input is (even with an input with only av1 and opus audio).

If I switch to "includes" mode, and set the codecs to "aac,ac3,vorbis" (implying that I only want to remove the stream types in the list), the flow plugin properly removes those streams without modifying the remaining streams in any way.

meulop commented 11 months ago

I think the logic for "not_includes" with multiple values is a bit off - effectively it checks each stream against each value in the list in turn, and will remove the stream if any of the tests fail. So in your case I think it will test the video stream and remove because its codec_name isn't "opus", and then will test the audio stream and remove because the codec_name isn't "av1".

UnknownWitcher commented 11 months ago

The issue is that the developer has it looping through the input values and compares each value to each stream property value, so if you have more than one value it results in all streams being removed.

I modified the code and have it placed in my LocalFlowPlugins folder, I don't know how to make the changes on github for @HaveAGitGat to approve it, but I explained in my post how to fix the problem.

quadcom commented 2 months ago

The issue is that the developer has it looping through the input values and compares each value to each stream property value, so if you have more than one value it results in all streams being removed.

I modified the code and have it placed in my LocalFlowPlugins folder, I don't know how to make the changes on github for @HaveAGitGat to approve it, but I explained in my post how to fix the problem.

@UnknownWitcher I'm not quite following on what I need to do to get the fixed version in the LocalFlowPlugins folder. Can you explain a bit more. I'm running in a Docker env btw.

UPDATE: I've edited and re-edited the JS three times carefully following the instructions and I keep getting a read error. I'm messing it up somehow but I can't see where.

What I did notice is your fix is for a different branch. Running off the Master, that file is 105 lines whereas the branch you reference has 103 lines. I've tried to line up your replacement code with the Master branch file but its breaking it somehow. Updating Community Plugins will pull from the Master branch.

Your Link https://github.com/HaveAGitGat/Tdarr_Plugins/blob/cfc630d55ea9cb24ee8e5434a36503d7b6e235e8/FlowPlugins/CommunityFlowPlugins/ffmpegCommand/ffmpegCommandRemoveStreamByProperty/1.0.0/index.js#L80-L94

Master Branch https://github.com/HaveAGitGat/Tdarr_Plugins/blob/master/FlowPlugins/CommunityFlowPlugins/ffmpegCommand/ffmpegCommandRemoveStreamByProperty/1.0.0/index.js#L80-L94

UnknownWitcher commented 2 months ago

@quadcom you can grab this version which has the code I shared to solve the problem. Thank you @rgreen83 for pushing this fix.

Some Instructions: In your /Plugins/FlowPlugins/LocalFlowPlugins, you want to create the plugin folder with a version slightly higher than the community one, /ffmpegCommandRemoveStreamByProperty/1.0.1, then place the index.js from the mentioned link into that folder, make sure you run a few files through to test it works.

quadcom commented 2 months ago

@UnknownWitcher Hey thanks for jumping in.

So I DL'd that file and did what you said. But I kept getting an error in the localFLows section in the Library. Chekcing the container logs and I see the error.

}
[2024-09-11T15:06:40.503] [ERROR] Tdarr_Server - Error: Cannot find module '/app/server/Tdarr/Plugins/FlowPlugins/LocalFlowPlugins/ffmpegCommandRemoveStreamByProperty/1.0.1/index.js/index.js'
Require stack:
- /app/Tdarr_Server/srcug/plugins/noop.js
at Module._resolveFilename (node:internal/modules/cjs/loader:1140:15)
at resolveFileName (/app/Tdarr_Server/node_modules/resolve-from/index.js:29:39)
at resolveFrom (/app/Tdarr_Server/node_modules/resolve-from/index.js:43:9)
at module.exports (/app/Tdarr_Server/node_modules/resolve-from/index.js:46:41)
at module.exports (/app/Tdarr_Server/node_modules/import-fresh/index.js:14:19)
at searchFlowPlugins (/app/Tdarr_Server/srcug/plugins/flowPluginFuncs.js:1:2684)
at async /app/Tdarr_Server/srcug/api/mainApi.js:1:8726{
"code": "MODULE_NOT_FOUND",
"requireStack": [
"/app/Tdarr_Server/srcug/plugins/noop.js"
]

Why it has the path /index.js/index.js makes no sense to me at all! image

UnknownWitcher commented 2 months ago

@quadcom not sure what's going on with that, in this situation, I'd have to look through @rgreen83 commits to figure out what else they've done unless they know what's going on and could maybe help, however, to get around this, I've uploaded my index.js which I know works (currently using it as we speak).

https://gist.github.com/UnknownWitcher/edee3de3d7504829bf25f410273111a1

Edit: Here's an example of me using it specifically for subtitles, you can select the stream type (All, Video, Audio, Subtitles). In my flow, I use this plugin twice to remove audio/subtitle languages that I don't need.

image

quadcom commented 2 months ago

@UnknownWitcher, thanks for sending that up. I checked both files and there is a significant difference between the two. I suspect most of that is due to your addition of the stream types filtering.

Oddly enough, even your version threw up the same error message.

I resolved the error in the most hacky way possible. I created an "index.js" folder inside 1.0.1 and placed the index.js file into it. Basically, I put the file where it said it was looking for it.

This tells me that there is a bug elsewhere in the source code that is not related to this specific issue. It just so happened to show up now.

Anywho, your JS file is recognized, and I will be running some tests.

Thanks again, so very much!!

quadcom commented 2 months ago

Well I just filed a bug report as the problem runs deeper.

https://github.com/HaveAGitGat/Tdarr/issues/1078

UnknownWitcher commented 2 months ago

@quadcom I figured out the issue. You left out "ffmpegCommand" in your path, which is why both scripts had an issue.

It should be

/FlowPlugins/LocalFlowPlugins/ffmpegCommand/ffmpegCommandRemoveStreamByProperty/1.0.1/index.js

This was my fault, I forgot about that extra folder. Sorry about that.

quadcom commented 2 months ago

That fixed it. Not to worry. I'll kill the bug report.