Javacv: Frame Grabber is Crashing for Corrupt Videos.

[AKMANSoft]

Hi

I have a corrupt video, which is crashing my application, when capture frame from Frame Grabber.

Sample Code:

`import java.awt.image.BufferedImage;
import java.io.File;

import org.bytedeco.javacpp.avutil;
import org.bytedeco.javacv.FFmpegFrameGrabber;
import org.bytedeco.javacv.Frame;
import org.bytedeco.javacv.FrameGrabber.Exception;
import org.bytedeco.javacv.Java2DFrameConverter;

import com.msv.model.Video;

public class CaptureFrames {

public CaptureFrames() {

}

public static BufferedImage captureFrame(File video) throws Exception {
    return captureFrame(video, 1l);
}

public static BufferedImage captureFrame(File video, long time) throws Exception{

    BufferedImage convertedImage = null;

    avutil.av_log_set_level(avutil.AV_LOG_QUIET);

    FFmpegFrameGrabber grabber = new FFmpegFrameGrabber(video);

    grabber.setOption("max_nal_size", "1024");

    Java2DFrameConverter converter = new Java2DFrameConverter();

    grabber.start();

    grabber.setTimestamp(time * 1000);

    Frame frame = null;
    frame = grabber.grabFrame(false,true,true,false);

   // long totalTime = grabber.getLengthInTime();

    //System.err.println("total time is "+totalTime/ 1000000);

    if (frame != null && frame.image != null && frame.imageChannels!= 0) {
        if (converter.convert(frame) == null)
            return convertedImage;
        convertedImage = converter.convert(frame);

        return convertedImage;

    }

    grabber.stop();

    return convertedImage;
}

public static void main(String[] args) {

    try {
        CaptureFrames.captureFrame(new File("C:\\Users\\Admin\\Desktop\\CorruptVids\\_carved_019259.flv"));
    } catch (Exception e) {
        e.printStackTrace();
    }
}

}

Corrupt Video:
Attachment is a corrupt video, which is zip.

_carved_019259.zip
`
Crash logs:
hs_err_pid4624.log

[saudet]

Could you also attach a sample hs_err_pid*.log file?

[AKMANSoft]

I have attached the crash logs also.

[AKMANSoft]

Any idea how to resolve this bug?

[saudet]

It's probably just a check missing somewhere in processImage()...

[AKMANSoft]

I have change the code, I just want to grab the image in my case, so i remove the Audio Part from it.

Also in process image method, I have found that
frame.image[0].limit(frame.imageHeight * frame.imageStride);
was giving exception and after that app immediate crash.

I have checked that exception was buffered Capacity is less then imageHeight * imageStride.

I have put a check, but crash still happening.

 if((frame.imageHeight * frame.imageStride)> frame.image[0].capacity()) {
        throw new IllegalArgumentException();
    }

    frame.image[0].limit(frame.imageHeight * frame.imageStride);
    frame.imageChannels = frame.imageStride / frame.imageWidth;

The new err logs is also attached.
hs_err_pid7996.log

[saudet]

It's crashing in avformat_seek_file(). Please report this issue upstream! Thanks

[AKMANSoft]

Thank you for your support. What other checks we can put to avoid the crash.
I have added the check if the video length is zero then do not process the video. which work with this video case.
but i have few more corrupt videos which has video lengths and im able to extract few frames, but i have noticed when i stopped the grabber the app crash.
due you have suggestion about it.

[saudet]

It's possible FFmpeg is not expecting to have any other functions called after an error, so it might be a good ided to stop() the grabber when an error is encountered?

[AKMANSoft]

Thank you. I'm Sorry i didn't check the new version which resolve my problem. I have check the code of FFmpegFrameGrabber which has some modifications like FFmpege Locks. also the setTimeStamp method is modified but it increase the time to grab the frame, The previous method was getting frame more quickly.

[saudet]

@anotherche Could you put back the old quick and dirty seek function? Just with an additional optional boolean flag would be good I guess. Thanks!

[anotherche]

OK. I can do setTimeStamp(long timestamp) to call old method by default and setTimeStamp(long timestamp, bool checkFrames) to call the changed code with checkFrames=true (then setTimeStamp(long timestamp) will correspond to checkFrames=false). Is this appropriate?

[saudet]

I wonder which one should be the default, thoughts @AKMANSoft ?

[anotherche]

IDK which is better too. It depends on how bad (slow/buggy) is the new code of setTimestamp.

[saudet]

Hum, it can get pretty bad for corrupted streams I think. So, maybe we should have checkFrames=false as default, but make it clear to users that it should be set to true to get better accuracy.

[anotherche]

Well, am I correct that the new seek code solves @AKMANSoft's issue?

I'm Sorry i didn't check the new version which resolve my problem.

At least mine app using new setTimestamp cope well with that corrupted file attached by @AKMANSoft.
(The slowness mentioned by @AKMANSoft is interesting to me. How much slow is it compared to the original code?)
As well #841 is solved with #908 . So I'm starting to think checkFrames=true should be default...
Any way, it will be quite easy to change our selection later. But it should be done before a release version.

[anotherche]

I have committed the code with old method by default. But actually I had to introduce a minor modification to that old seeking code. Without this modification, the first while loop makes those 1000 steps sometimes even with not corrupted files. This occurs because avformat_seek_file() sometimes decides to set grab position just ahead the required timestamp. In this case that
while (this.timestamp > timestamp + 1 && grabFrame(true, true, false, false) != null && count++ < 1000) {}
runs away till the count ==1000
So, to prevent this situation we should check if grabFrame returns a frame with data. like this

while (this.timestamp > timestamp + 1 && grabFrame(true, true, false, false) != null && count++ < 1000) {
// flush frames if seeking backwards
if (frame.image != null || frame.samples != null) break; // stop flushing if a frame containing data was grabbed
}

[saudet]

@anotherche Sounds good, send a pull request!

If I understand correctly, @AKMANSoft was referring to the crash he was observing being fixed with the latest version of FFmpeg, which in unrelated to the issue with the slowness of the seek.

[AKMANSoft]

Hi, Sorry for a delay response, In my app i have to grab frame very quickly, Imagine the frame we grab is showing on screen and user move his mouse, with mouse movement i have to grab frame, if the user move mouse very quickly then i have to grab the frame quickly to give the user a smooth experience.
The new method of setTimestamp is slow and its not smooth, while the old method is much faster and give the user a very smooth experience, so the default one will be with check frame = false is the good option for me.

Also @saudet you are right, my problem was, the app was crashing for corrupt videos which resolve in this version due to i think extra checks and locking on ffmpeg, but grabbing frame was slow, so i have to replace it with old method.

I think we can create a test for this method to grab all the frames of the video and check the time with the old and new method?

@anotherche good fix for count++ < 1000 for corrupt video, but why you put 1000? can you explain?

[anotherche]

@AKMANSoft, this count check was actually not by me. It was introduced in v. 1.4.
There was no count check in v. 1.3.3 and earlier.
Without the count check the seeking in corrupted files (and sometimes even in good files) may hang in the while loop till the end of a file. So, it was introduced since 1.4 to fight these accidents of long while loop. But why this should be 1000? I think it was a quick simple solution for JavaCV could support bad situations (bad files) more or less adequately.
The story is even more interesting though. Setting certain limiting number of counts in the first while loop and in the second while loop in the code of setTimestamp has different consequences. In the first loop count++<1000 limits number of grabs which erroneously leads us to the end of the file. So we should simply stop this grabbing somewhere. It is probably that even 100 or even 10 steps is enough to stop this loop, I believe. Because actually avformat_seek_file() function used in the setTimestamp should do its work so that the next grab position will be close to the required and it should be slightly before the requested timestamp in most cases. So the first while loop is only necessary to flush occasional empty frames and to reach a frame containing video or audio data which will have timestamp < requested (but as I said, sometimes avformat_seek_file() select a position slightly after the requested timestamp - within a couple of frames, and then we'd better should stop this loop right there. I've made this change in the loop). I believe that in most normal cases there should be only few occasional empty frames after call to avformat_seek_file() function. That's why I think the limit of first loop can be set 10 (or 20, or 30 I do not know - the optimal limit should be a compromise between normal and bad situations).
As for the limiting count for the second while loop. The second loop is necessary to set grab position as close to the requested timestamp as possible. And sometimes it requires to do many grabs (and sometimes even greater than 1000). It is because that avformat_seek_file() function again. This function sets the decoding to a closest key frame. And sometimes there is long distance between key frames. Say, after avformat_seek_file() call we may found ourselves at a key frame located 500 frames before the desired (and this is only number of video frames). If the file contains audio, then there can be, say, 800 audio frames on the way from this key frame to the required timestamp. As setTimestamp should grab both types of frames (video and audio) this case will give us 1300 steps necessary to finish the seeking. So, even 1000 can be not enough sometimes for the second while loop.
So all these facts forced me to start changing setTimestamp function to make it more precise and predictable. Of course, the resulting code is heavier. But not so much, I believe. Moreover, I thought that the seeking in files is not very often procedure in most application. I understand your type of application. While changing the code I also checked that the seek should be smooth enough, and it was quite satisfactory for me. But maybe you need more smooth action, or maybe we compare different types of hardware. I checked it on a normal average desktop, although I understand that there wide range of hardware JavaCV can be used, including smarts.
By the way, in case of corrupted files the seeking can be slow even with the old code because of that count++<1000.
While I was writing this long story an idea came to me how to make truly fast and rough seeking in long video. If you are not interested in precise positioning while scrolling through a file, we may remove the second while loop at all and stop after a first non-empty frame grabbed in the first while loop (this can be a fast variant of SetTimestamp function). Then you do grabImage in the application code which should put the position approximately within several seconds close to the requested timestamp. If you are not satisfied the distance between resulting frame and the desired frame, you can do several grabs more (the setTimestamp does it for you anyway). And finally you position the scroll according the actual timestamp.

[saudet]

@AKMANSoft Could try the patch from @anotherche (pull #949) and let us know if it works well?

[saudet]

Any updates @AKMANSoft ?

[anotherche]

@saudet, I was trying to build an app against the shapshot to check the performance of seeking, but a dependency error appears on running the code.
When I use

<dependency>
    <groupId>org.bytedeco</groupId>
    <artifactId>javacv-platform</artifactId>
    <version>1.4.1-SNAPSHOT</version>
</dependency>

the error appears when I try to use FFmpegFrameGrabber in the app

Exception in thread "AWT-EventQueue-0" java.lang.NoClassDefFoundError: org/bytedeco/javacpp/Pointer

There is no error when I use a release version like

<dependency>
    <groupId>org.bytedeco</groupId>
    <artifactId>javacv-platform</artifactId>
    <version>1.4.1</version>
 </dependency>

The build finishes without errors in both cases. I use dependencies provided by mvn dependency:copy-dependencies and placed right in the application folder.
So, how to build against snapshot versions?

[saudet]

You'll need 1.4.2-SNAPSHOT, not 1.4.1, right?

[anotherche]

OK. then with 1.4.2-SNAPSHOT I have

java.lang.NoClassDefFoundError: org/bytedeco/javacv/Java2DFrameConverter

[saudet]

Could you post the full stack trace?

[anotherche]

Exception in thread "AWT-EventQueue-0" java.lang.NoClassDefFoundError: org/bytedeco/javacv/Java2DFrameConverter
        at ffmpeggrabber_seektest.mainwnd.<init>(mainwnd.java:34)
        at ffmpeggrabber_seektest.mainwnd$1.run(mainwnd.java:43)
        at java.awt.event.InvocationEvent.dispatch(Unknown Source)
        at java.awt.EventQueue.dispatchEventImpl(Unknown Source)
        at java.awt.EventQueue.access$500(Unknown Source)
        at java.awt.EventQueue$3.run(Unknown Source)
        at java.awt.EventQueue$3.run(Unknown Source)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(Unknown Source)
        at java.awt.EventQueue.dispatchEvent(Unknown Source)
        at java.awt.EventDispatchThread.pumpOneEventForFilters(Unknown Source)
        at java.awt.EventDispatchThread.pumpEventsForFilter(Unknown Source)
        at java.awt.EventDispatchThread.pumpEventsForHierarchy(Unknown Source)
        at java.awt.EventDispatchThread.pumpEvents(Unknown Source)
        at java.awt.EventDispatchThread.pumpEvents(Unknown Source)
        at java.awt.EventDispatchThread.run(Unknown Source)
Caused by: java.lang.ClassNotFoundException: org.bytedeco.javacv.Java2DFrameConverter
        at java.net.URLClassLoader.findClass(Unknown Source)
        at java.lang.ClassLoader.loadClass(Unknown Source)
        at sun.misc.Launcher$AppClassLoader.loadClass(Unknown Source)
        at java.lang.ClassLoader.loadClass(Unknown Source)
        ... 16 more

line 34 is

final Java2DFrameConverter converter = new Java2DFrameConverter();

[saudet]

Caused by: java.lang.ClassNotFoundException: org.bytedeco.javacv.Java2DFrameConverter

JavaCV isn't in your class path.

[anotherche]

You are right. The problem is in the naming. All the dependencies copied by mvn dependency:copy-dependencies have "-SHAPSHOT" in their names, but the classpath in the application's MANIFEST.MF
uses timestamps instead the SNAPSHOT. Like that: javacv-1.4.2-20180403.015509-4.jar.
Please, be kind to me.. tell me how to fight this? I wonder, why all these things are not automatic...

[anotherche]

I've copied the correct files by hands.. It works, thanks. But anyway, there should be normal way to automate this.

[saudet]

I'm 100% sure the following works.

  <dependency>
    <groupId>org.bytedeco</groupId>
    <artifactId>javacv-platform</artifactId>
    <version>1.4.2-SNAPSHOT</version>
  </dependency>

[saudet]

I've merged pull #949 from @anotherche (thanks for the help!), so this should now be fixed. @AKMANSoft Please try the latest snapshots and let us know if you still have issues:
http://bytedeco.org/builds/

[anotherche]

I've done some tests with rate of random seeking using different builds: 1.3.3, 1.4, 1.4.1, and 1.4.2-SNAPSHOT
I did not have much time, so not many test were done. It was two avi films (DivX3 960kbps and XviD 1500 kbps) and two mkv films (AVC High@L4 11ref frames and AVC [email protected] 9 ref frames).
The test was like this:
1000 times of (random seek + grabImage + java.awt.Graphics.drawImage in a window).
General conclusion: all the builds were more or less similar in performance. The performance depends mostly on the encoder used.
1.3.3 was slightly faster with avi sources (0.033s +- 0.022s) per frame.
1.4, 1.4.1, 1.4.2-snapshot performed almost equally (0.045s +- 0.03s)
So 1.3.3 was ~30% faster in operations. But it was often hanging during tests.
I suppose that the difference is connected with ffmpeg or something other, but not with the seeking itself, as 1.4 is seeking code is similar to 1.3.3 while 1.4.1 is the new one, and 1.4.2-snapshot is again similar to 1.4 and 1.3.3.
The seeking in mkv's is slower in all the cases and it does not depend on the build ~(0.145s +- 0.11s) for the simpler encoder settings and ~(0.35s+-0.25s) for the higher.

[saudet]

Duplicate of #991

[AKMANSoft]

Sorry for delayed response, and Thank you @anotherche for the testing, 30% delay is very much in my case, and user extract the images in realtime if they move mouse quickly, they feel a considerable delay in the new version, we need to keep open the previous method for setTime.

@saudet at last i find the issue which was causing the app to be crash. I'm only using three methods.
setTime(long timeStamp);
grabber.grab();
grabber.stop();

The these methods is called in the multi threaded environment. So if the setTime(long timeStamp); and in the same time grabber.stop(); is called, the app was crashing, So i put those three methods in RentranceLock, that if one is working then other should wait for it. It solve my problem.

[saudet]

@AKMANSoft Please add your comments to issue #991!