{"action":"opened","issue":{"url":"https://api.github.com/repos/WritingMinds/ffmpeg-android-java/issues/198","repository_url":"https://api.github.com/repos/WritingMinds/ffmpeg-android-java","labels_url":"https://api.github.com/repos/WritingMinds/ffmpeg-android-java/issues/198/labels{/name}","comments_url":"https://api.github.com/repos/WritingMinds/ffmpeg-android-java/issues/198/comments","events_url":"https://api.github.com/repos/WritingMinds/ffmpeg-android-java/issues/198/events","html_url":"https://github.com/WritingMinds/ffmpeg-android-java/issues/198","id":224172901,"number":198,"title":"Inadequate locking on execute","user":{"login":"lbdroid","id":3372230,"avatar_url":"https://avatars1.githubusercontent.com/u/3372230?v=3","gravatar_id":"","url":"https://api.github.com/users/lbdroid","html_url":"https://github.com/lbdroid","followers_url":"https://api.github.com/users/lbdroid/followers","following_url":"https://api.github.com/users/lbdroid/following{/other_user}","gists_url":"https://api.github.com/users/lbdroid/gists{/gist_id}","starred_url":"https://api.github.com/users/lbdroid/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lbdroid/subscriptions","organizations_url":"https://api.github.com/users/lbdroid/orgs","repos_url":"https://api.github.com/users/lbdroid/repos","events_url":"https://api.github.com/users/lbdroid/events{/privacy}","received_events_url":"https://api.github.com/users/lbdroid/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-04-25T15:12:04Z","updated_at":"2017-04-25T15:12:04Z","closed_at":null,"body":"Trying to execute several times in rapid succession results in failure to detect the running state of ffmpeg.\r\n\r\nI believe that, FIRST of all, execute should be *synchronized*,\r\n\r\nSECONDLY, Util.isProcessCompleted() has bad logic; specifically, (process == null) until Runtime.getRuntime().exec(commandString) and subsequently ShellCommand.run, actually return. It is possible for it to be called BEFORE process is instantiated, which would cause bad things to happen.\r\n\r\nTHIRD, FFmpegExecuteAsyncTask.isProcessCompleted() is altogether pointless, since it would be preferable to use the value of FFmpegExecuteAsyncTask.getStatus(), which will return one of AsyncTask.Status.PENDING, AsyncTask.Status.RUNNING, or AsyncTask.Status.FINISHED.\r\n\r\nFOURTH, shellCommand.run(cmd) probably should not be called in AsyncTask.doInBackground,  but rather in AsyncTask.execute.\r\n\r\nTo fix the worst of the problem;\r\n\r\n1) public *synchronized* void execute(...)\r\n2) FFmpegExecuteAsyncTask.isProcessCompleted(){ return this.getStatus() == AsyncTask.Status.FINISHED; } --- or even better, just get rid of that function altogether. It is, after all, useless.\r\nInstead of ffmpegExecuteAsyncTask.isProcessCompleted(), use (ffmpegExecuteAsyncTask.getStatus() == AsyncTask.Status.FINISHED)\r\n\r\n\r\nA further complaint I have about this is that AsyncTask isn't meant for this type of usage. From the documentation;\r\n\"AsyncTasks should ideally be used for short operations (a few seconds at the most.) If you need to keep threads running for long periods of time, it is highly recommended you use the various APIs provided by the java.util.concurrent package such as Executor, ThreadPoolExecutor and FutureTask.\"\r\n\r\nFFmpeg violates this, since an ffmpeg process can run not just for \"a few seconds at the most\", but hours or DAYS."}}
{"actor":{"display_login":"lbdroid"}}