Reduced deprecated code in README.md

[gertjanal]

The current tutorial code has deprecated methods. Replaced them with the new ones. Only one .codec() is left, unsure how to fix.

[saudet]

The idea is to have some sample translated code that is based on an existing version in C/C++. Here, tutorial01.c happens to be using deprecated functions...

Anyway, not a bad idea to update it. But before we merge this, could you at least remove the try block there? It doesn't do anything useful. Thanks!

[gertjanal]

The try block makes sure the finally with the cleanup is always called, even when exiting with return or an throw new Exception

[saudet]

Yes, your reasoning is good, but we don't need a try-with-resources on AVPacket. We could use one inside SaveFrame() though on the OutputStream. Could you modify it that way? Thanks!

[gertjanal]

Added try-with-resources to SaveFrame()

Perhaps we don't need the try-with-resources on AVPacket, but the try-finally makes sure this code is always called when done or in case of return or exception:

// Free the RGB image
av_free(buffer);
av_free(pFrameRGB);

// Free the YUV frame
av_free(pFrame);

// Close the codec
avcodec_close(pCodecCtx);

// Close the video file
avformat_close_input(pFormatCtx);

[saudet]

Yes, this is better, thank you!

One last thing: You're mixing up pFrame and pFrameRGB. Please change it back to how it was originally. Also, don't initialize their fields because they get initialized by the functions called.

[gertjanal]

Are you sure? The code doesn't work anymore when I don't initialize the fields. Can you copy-paste the code with the intended changes?

(or steal this merge request)

[saudet]

Well, the way it is now doesn't work for me. Executing the following command:

mvn clean compile exec:java -Dexec.args=http://fate-suite.ffmpeg.org/h264/bbc2.sample.h264

with this pom.xml file:

<project>
    <modelVersion>4.0.0</modelVersion>
    <groupId>org.bytedeco.javacpp-presets.ffmpeg</groupId>
    <artifactId>tutorial01</artifactId>
    <version>1.3</version>
    <properties>
        <exec.mainClass>Tutorial01</exec.mainClass>
        <maven.compiler.target>1.7</maven.compiler.target>
        <maven.compiler.source>1.7</maven.compiler.source>
    </properties>
    <dependencies>
        <dependency>
            <groupId>org.bytedeco.javacpp-presets</groupId>
            <artifactId>ffmpeg-platform</artifactId>
            <version>3.2.1-1.3</version>
        </dependency>
    </dependencies>
</project>

and your Tutorial01.java, I get the following output:

Input #0, h264, from 'http://fate-suite.ffmpeg.org/h264/bbc2.sample.h264':
  Duration: N/A, bitrate: N/A
    Stream #0:0: Video: h264 (High), yuv420p(tv, bt709, top first), 1440x1080 [SAR 4:3 DAR 16:9], 25 fps, 25 tbr, 1200k tbn, 50 tbc
[swscaler @ 0x7efd09909bc0] bad dst image pointers
[WARNING] 
java.lang.reflect.InvocationTargetException
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.codehaus.mojo.exec.ExecJavaMojo$1.run(ExecJavaMojo.java:294)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NullPointerException
	at Tutorial012.saveFrame(Tutorial012.java:40)
	at Tutorial012.main(Tutorial012.java:141)
	... 6 more

At the very least please make it work! Thank you

[gertjanal]

OOPS! So sorry. I see what you mean with You're mixing up pFrame and pFrameRGB. 😄 pushed working version :)

[saudet]

Better, now it works :) Thanks

If we really want to use try blocks and stuff though, let's use them properly. The one in the main() method should start before the call to avformat_open_input(), and so all variables need to be declared and initialized at the top, just like in the original C code.

Less importantly, the Exception throws that you added seem conceptually closer to IOException than IllegalStateException, since they can occur often and depend on the input. What do you think?

[saudet]

Actually, your updated code still uses deprecated API. AFAIU, the following line will not compile starting from the next major version of FFmpeg:

final AVCodecContext pCodecCtx = videoStream.codec();

I couldn't find a properly updated version of that sample in C. Maybe it would be simpler to use an example that comes with FFmpeg itself? This one looks like a good candidate, but it's a bit long:
https://www.ffmpeg.org/doxygen/trunk/demuxing_decoding_8c-example.html
What do you think?

[gertjanal]

I think my code suggestion is still a good improvement. However incomplete, it has less deprecated code than the current example.

Also, I have think this takes too much of our time, mostly because I'm not an FFmpeg expert. You can close this pull request if you can think it is incomplete, or merge it if it is an improvement (and create an issue for further improvement)

Thanks for your time and support!

[saudet]

I agree that we should get rid of deprecated code, but it needs to work. And the purpose of the example in Java is also to provide users with an equivalent translation of an existing example in C/C++.

This example appears to be an updated version of the old one without any deprecated calls:
https://github.com/monday0rsunday/ffmpeg-tutorial/blob/master/002_read_few_frame.c
If you could take this and translate it to Java, that would be awesome!