{"action":"created","issue":{"url":"https://api.github.com/repos/bytedeco/javacv/issues/908","repository_url":"https://api.github.com/repos/bytedeco/javacv","labels_url":"https://api.github.com/repos/bytedeco/javacv/issues/908/labels{/name}","comments_url":"https://api.github.com/repos/bytedeco/javacv/issues/908/comments","events_url":"https://api.github.com/repos/bytedeco/javacv/issues/908/events","html_url":"https://github.com/bytedeco/javacv/pull/908","id":296267470,"number":908,"title":"Change truncation to rounding to estimate frameNumber; Improve backward seeking","user":{"login":"anotherche","id":9344635,"avatar_url":"https://avatars3.githubusercontent.com/u/9344635?v=4","gravatar_id":"","url":"https://api.github.com/users/anotherche","html_url":"https://github.com/anotherche","followers_url":"https://api.github.com/users/anotherche/followers","following_url":"https://api.github.com/users/anotherche/following{/other_user}","gists_url":"https://api.github.com/users/anotherche/gists{/gist_id}","starred_url":"https://api.github.com/users/anotherche/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/anotherche/subscriptions","organizations_url":"https://api.github.com/users/anotherche/orgs","repos_url":"https://api.github.com/users/anotherche/repos","events_url":"https://api.github.com/users/anotherche/events{/privacy}","received_events_url":"https://api.github.com/users/anotherche/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2018-02-12T05:05:27Z","updated_at":"2018-02-17T21:16:01Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/bytedeco/javacv/pulls/908","html_url":"https://github.com/bytedeco/javacv/pull/908","diff_url":"https://github.com/bytedeco/javacv/pull/908.diff","patch_url":"https://github.com/bytedeco/javacv/pull/908.patch"},"body":"So, there are two topics in this request.\r\nThe first is quite simple - current code of estimation of frameNumber (made in `public Frame grabFrame(boolean doAudio, boolean doVideo, boolean processImage, boolean keyFrames)`) and `getLengthInFrames()` use simple truncation of the result like \r\n```\r\n(int)(timestamp * getFrameRate() / 1000000L)\r\n```\r\nor\r\n```\r\n(int)(getLengthInTime() * getFrameRate() / 1000000L)\r\n```\r\nThis truncation gives incorrect results if the framerate is not a divisor of 1000000. In this case, the frameNumber is not increasing monotonously and the total number of frames in the video is reduced by one. The problem is solved by changing of the truncation to rounding.\r\n\r\nThe second topic is about seeking  backwards. The current code use the following while loop in `public void setTimestamp(long timestamp)` function:\r\n```\r\nint count = 0; // prevent infinite loops with corrupted files\r\nwhile (this.timestamp > timestamp + 1 && grabFrame(true, true, false, false) != null && count++ < 1000) {\r\n                // flush frames if seeking backwards\r\n}\r\n```\r\nIf I'm not wrong, this loop looks superfluous in all cases of normal not corrupted files. I think so because the preceding code should guarantee reposition of the decoding place just before the necessary seek and flushing of all internal buffers. Thus, the first successful call of ` grabFrame(true, true, false, false)` should grab a frame with `this.timestamp <= timestamp +1`. It means that, in case of not corrupted files, the while loop will always run exactly once when seeking backwards.  Thus it can be replaced with a single conditional call of the `grabFrame(true, true, false, false)` like \r\n```\r\nif (this.timestamp > timestamp + 1) grabFrame(true, true, false, false);\r\n```\r\nThis change, if it will not break the seeking logic, will additionally improve a situation with some cases of not so good files. I mean that if the previous call of \r\n```\r\navformat_seek_file(oc, -1, Long.MIN_VALUE, timestamp, Long.MAX_VALUE, AVSEEK_FLAG_BACKWARD)\r\n```\r\nwill not position the decoding to a point **before** the desired timestamp, but position it somewhere **after**, the discussed while loop will bring us at least 1000 frames ahead. If to change the while loop to the conditional grab we will stay closer to the desired seek. I checked the behavior in several tests with movies encoded with various encoders. It seems that the suggested code works well (although I only looked at video frames, not audio samples, but I think there should not be a problem (???)). And it prevent wrong seeking (seeking to 1000 frames ahead) in case of video files encoded with XviD with \"packed b-frames\" option (which is is not so rear case)."},"comment":{"url":"https://api.github.com/repos/bytedeco/javacv/issues/comments/366471977","html_url":"https://github.com/bytedeco/javacv/pull/908#issuecomment-366471977","issue_url":"https://api.github.com/repos/bytedeco/javacv/issues/908","id":366471977,"user":{"login":"anotherche","id":9344635,"avatar_url":"https://avatars3.githubusercontent.com/u/9344635?v=4","gravatar_id":"","url":"https://api.github.com/users/anotherche","html_url":"https://github.com/anotherche","followers_url":"https://api.github.com/users/anotherche/followers","following_url":"https://api.github.com/users/anotherche/following{/other_user}","gists_url":"https://api.github.com/users/anotherche/gists{/gist_id}","starred_url":"https://api.github.com/users/anotherche/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/anotherche/subscriptions","organizations_url":"https://api.github.com/users/anotherche/orgs","repos_url":"https://api.github.com/users/anotherche/repos","events_url":"https://api.github.com/users/anotherche/events{/privacy}","received_events_url":"https://api.github.com/users/anotherche/received_events","type":"User","site_admin":false},"created_at":"2018-02-17T21:16:01Z","updated_at":"2018-02-17T21:16:01Z","author_association":"NONE","body":"So, I improved the seeking code. The tests are going without errors now. I have checked it on various video files, as well as on that generated by the test. The test code is changed somewhat as well. I tried to add comments to every new parts of the code so that it should be self-explanatory more or less.\r\nAdditionally, I suggest to add some helper API useful for user applications: information about streams and format. "}}
{"actor":{"display_login":"anotherche"}}