Fix Bug #261 destroyProcessByID not working #263
Conversation
The ProcessBuilder() starts process for Python lib. Python lib further creates a process for ffmpeg if used for downloading. So the process returned by .start() method is only capable to kill child process(python) of the app not sub process(ffmpeg). As this(ffmpeg) belongs to the app tree, so we are able to kill it even android has restrctions to kill other's app process. Tree for process: App Process(PID) -Python Process(PID) --FFMPEG Process(PID) Now the issue was that if we use "destroyProcessById()" the FFMPEG process still continue to download the app. So we have to manually kill this as well. This patch will solve the bug.
|
Thanks for your PR! According to the code, it seems we're trying to terminate all the child processes when destroy a single yt-dlp process, this might not be the expected behavior if we want to allow multiple yt-dlp instance to be running at the same time |
|
Maybe we can try using |
library/src/main/AndroidManifest.xml
Outdated
There was a problem hiding this comment.
Do we really need this permission when killing our own processes? See also: https://developer.android.com/reference/android/Manifest.permission#KILL_BACKGROUND_PROCESSES
| @@ -120,6 +120,7 @@ object YoutubeDL { | |||
|
|
|||
| fun destroyProcessById(id: String): Boolean { | |||
| if (idProcessMap.containsKey(id)) { | |||
| killChildProcesses() | |||
There was a problem hiding this comment.
We should only terminate the child processes of the exact process to be destory
| fun killChildProcesses() { | ||
| val process = Runtime.getRuntime().exec("ps -A -o pid,cmd") | ||
| process.inputStream.bufferedReader().useLines { lines -> | ||
| lines.filter { it.contains("ffmpeg") } |
There was a problem hiding this comment.
Maybe aria2c too? Not sure about how many external executables in our libraies could have this issue
There was a problem hiding this comment.
See also: JunkFood02/Seal#180
There was a problem hiding this comment.
And this function should actually be named killAllFFmpegProcesses according to its implementation
This pathch will also issue of killing selective child process. The process is to get PID from process which we want to kill(via reflection). Next get child (ffmpeg) id for it and kill
|
I have successfully updated the code to kill only selective ffmpeg process. Kindly test and feed back me |
This patch will fix to kill ffmpeg specific child process and also get result from ffmpeg progress via callbacks
Previously, it was only showing a line for ffmpeg progress. Now to get downloaded size, i added a size param to the progress callback function
Some times(devices dependent), onComplete() is not triggered. So it is fixed
To detemine if video link is live streaming or not, is_live field is added to VideoInfo class
ytdlp is updated to the latest nightly version
Previously it was giving error of outofmemory and replaced with buffered reader but not working. Now wrapped with trycatch
Code optimized to handle memory exception
This bug is only found while using the library
The ProcessBuilder() starts process for Python lib. Python lib further creates a process for ffmpeg if used for downloading. So the process returned by .start() method is only capable to kill child process(python) of the app not sub process(ffmpeg). As this(ffmpeg) belongs to the app tree, so we are able to kill it even android has restrctions to kill other's app process.
Tree for process:
App Process(PID)
-Python Process(PID)
--FFMPEG Process(PID)
Now the issue was that if we use "destroyProcessById()" the FFMPEG process still continue to download the app. So we have to manually kill this as well.
This patch will solve the bug.
Close #261