FFmpegFrameGrabber.setVideoFrameNumber setting unexpected frames

[svorlauf]

version: since 1.5.5 (probably pull #1559 )

setVideoFrameNumber skips to a Frame that is not the expected one

example using a 30fps video:

frameGrabber = new FFmpegFrameGrabber(file); frameGrabber.start(); System.out.println("sequential:"); for (int i = 0; i < 10; i++) { long timestamp = frameGrabber.grabImage().timestamp; System.out.println("number:" + i + " timestamp:" + timestamp); } System.out.println("setVideoFrameNumber:"); for (int i = 0; i < 10; i++) { frameGrabber.setVideoFrameNumber(i); long timestamp = frameGrabber.grabImage().timestamp; System.out.println("number:" + i + " timestamp:" + timestamp); }

creates the output:
sequential: number:0 timestamp:0 number:1 timestamp:33333 number:2 timestamp:66666 number:3 timestamp:100000 number:4 timestamp:133333 number:5 timestamp:166666 number:6 timestamp:200000 number:7 timestamp:233333 number:8 timestamp:266666 number:9 timestamp:300000 setVideoFrameNumber: number:0 timestamp:0 number:1 timestamp:0 number:2 timestamp:66666 number:3 timestamp:100000 number:4 timestamp:100000 number:5 timestamp:166666 number:6 timestamp:200000 number:7 timestamp:200000 number:8 timestamp:266666 number:9 timestamp:300000

I already dug into the setTimestamp method. looks like the fraction of the frameDuration is not handled correctly.

[svorlauf]

Also setTimestamp looks very messy. I would like to write some unit tests for setVideoFrameNumber and setAudioFrameNumber, and extract a clean setTimestamp method for these. Thus the current (big) setTimestamp method can be cleaned up / redone when more specification (unit test) is available

By the way at the moment setVideoFrameNumber/setVideoTimestamp uses a fallback to audio timestamp when no video stream is available and vice versa. is this part of the expected behavior?

[saudet]

/cc @anotherche

[anotherche]

OK, I can see the cause of the problem. I have a suggestion on how to try to fix this, but first I will give my thoughts on the possibilities of using seeking by frame number. The fact is that the seeking by frame number never guarantees hitting the exact frame number, because it always relies on the timestamp, and not on the frame number (there is no information about frame numbers in video streams, we can only calculate their approximate time of presentation). Take this as an axiom. The simplest reason for inaccuracy is a rounding error. But there may also be more severe reasons - variable frame rate, incorrect PTSs, whatever else. So the error can vary from a few microseconds to a few frames. And even one microsecond error may result in "not correct" frame (the previous or the next). By the way, the above code example, when you go sequentially from frame to frame, has very little practical sense (after all, you can just read frames sequentially, right?).

But okay, I see a place in the code that is guaranteed to lead to the observed problem in precisely such a meaningless sequential seek method. In lines 758-761 we calculate maximal number of grabs to get to the requested timestamp. I suggest replacing two lines inside the conditional block and check the result:

if(frameDuration>0.0) {
     maxSeekSteps = 0; //no more grab if the distance to the requested timestamp is smaller than frameDuration
     if (timestamp - initialSeekPosition + 1 > frameDuration)  //allow for a rounding error
               maxSeekSteps = (long)(10*(timestamp - initialSeekPosition)/frameDuration);
}

I believe that in most cases of constant framerate video this should be sufficient for correct determination of boundaries between frames.

[svorlauf]

You are right, the code example has no practical use. It is only to demonstrate the difference in output for sequential grabbing and using the setVideoFrameNumber method. The real use case is some kind of video playback in opengl. So we actually jump in time and we do not want to only update an image to the gpu when the frame changes. thus we do our own time to frame calculation

[anotherche]

Yes, I believe it is correct when you know what you want to achieve and use your own algorithm for this task. Because for such a stream seeking, based on the frames indexes, it is almost impossible to create a universal code that suits everyone. For example, I can share a simple technique - if at a jump between two positions in the stream you move forward a small number of frames (within, say, 100), then it is more rational not to use the settimestamp or framenumber, but simply do sequential grabbing until the required place.
As for the behavior of the setVideoFrameNumber/setAudioFrameNumber in files with no video or audio streams - it really may seem strange. But this is such a case when it is really not clear what is the best way to proceed. How to react when user code tries to use these methods without checking streams. Yes, you can offer two other options - to throw an exception (let everyone deal with the checks themselves), or always return a zero frame. But I chose the third option - since in the previous versions of the graber no one paid attention at all by which frames we seek, I assumed that in such circumstances it was possible to imitate the "old" behavior - to seek regardless of the frames type.

[svorlauf]

I am fine with the fallback to the other stream in setTimestamp. Like you said you had to choose one of the options. And I failed to see that setTimestamp is not even called by setVideoFrameNumber if no video stream is available..
The whole setVideoFrameNumber/setAudioFrameNumber relies on a constant framerate when calculating the timestamp from frameNumber. As I understand the setTimestamp method is also able to handle variable bitrate streams.
IMHO setFrameNumber and setTimestamp look like two different use cases. So maybe the best way is to leave the setTimestamp method as it is and create a simple implementation for the setFrameNumber methods that implies a constant framerate. Then every timestamp calculated in the setFrameNumber methods should be mappable to one frame each in the new method

[saudet]

Thanks to @anotherche, this should be fixed with pull #1734.
Please give it a try with the snapshots: http://bytedeco.org/builds/

[saudet]

The fix has been released with JavaCV 1.5.7, enjoy! Again, thank very much @anotherche!
I'm assuming that this is all working correctly now, but please let me know if this isn't the case.