First ffmpeg reader #928

mikaelsundell · 2014-08-05T21:20:16Z

Promised 2 years ago at Siggraph to push my oiio/libquicktime reader :-) finally had som time to put together a more portable ffmpeg version. This is a first version and I don't consider final as-is.

lgritz · 2014-08-05T21:42:46Z

At first glance, this looks very cool! (I haven't even tried to compile, I'll do a more thorough examination when I get more time.)

Can you fill us in a little on the expected behavior? What formats should this be able to read? Does a movie file just look like a multi-image file to an OIIO-using app? What metadata does it set? I saw that you set "oiio:Movie" to nonzero to indicate that's it's a movie. How about metadata indicating the total number of frames (if known) and the fps? Are there other relevant data that are important?

mikaelsundell · 2014-08-05T22:38:26Z

Will do some more work on it tomorrow, need to make sure it runs as expected on all types of containers. My plan is to detect the supported containers (fileformats) and codecs using the underlying ffmpeg library, see: https://www.ffmpeg.org/ffmpeg-codecs.html

The reader itself acts like a multi-image file to OIIO apps, correct.
For the moment we only set a very basic set of metadata, fps and start/end/total number of frames is easy to add.
oiio::Movie indicates that seek_subimage can be used to read a frame, m_nsubimages = total number of frames.
So it doesn't add anything to the existing API this is up to the OIIO app to handle, example:
bool isMovie()
{
return getattribute("oiio::isMovie")
}
a writer will also be added very soon, it's not a big thing.

The main purpose for the ffmpeg reader/writer is to provide OIIO users with the ability to produce quicktime/mp4/avi files from sequences. I know that some users don't like the idea but audio would be useful to have in oiio, or at least some sort of work-around with user-data for this.

Will keep commiting updates, we can also have a chat about it at Siggraph. Will join Beer of a feather this year!

Changed metadata handling, now copies all tags in dictionary Fixed incorrect reading of subframes, now reads until finished

mikaelsundell · 2014-08-06T21:15:01Z

Fixed metadata handling and general reading. Now tested with a number of .mov files found in the wild, will add a writer and make sure it appends subimages correctly.

WIP so far.

lgritz · 2014-08-23T19:26:36Z

src/ffmpeg.imageio/ffmpeginput.cpp

+public:
+    FFmpegInput ();
+    virtual ~FFmpegInput();
+    virtual const char *format_name (void) const { return "mov"; }


Do you think "mov" is the right 'format name'?

I think we should either call it "movie", or we should actually try to print the true format of the opened file (and "movie" if no file is currently opened or it can't tell).

lgritz · 2014-08-23T19:32:23Z

I applied this patch on my end to make 'iinfo -v' and 'oiiotool -v' work better (but also needing the change on your end with "oiio::Movie" -> "oiio:Movie"). I will also commit it to master, so if you rebase your changes on the current master, you'll also pick them up.

diff --git a/src/iinfo/iinfo.cpp b/src/iinfo/iinfo.cpp
index 8c54416..552bf43 100644
--- a/src/iinfo/iinfo.cpp
+++ b/src/iinfo/iinfo.cpp
@@ -564,6 +564,7 @@ print_info (const std::string &filename, size_t namefieldlength,
         printf ("\n");
     }

+    int movie = spec.get_int_attribute ("oiio:Movie");
     if (verbose && num_of_subimages != 1) {
         // info about num of subimages and their resolutions
         printf ("    %d subimages: ", num_of_subimages);
@@ -573,6 +574,8 @@ print_info (const std::string &filename, size_t namefieldlength,
                 printf ("%dx%dx%d ", spec.width, spec.height, spec.depth);
             else
                 printf ("%dx%d ", spec.width, spec.height);
+            if (movie)
+                break;
         }
         printf ("\n");
     }
diff --git a/src/oiiotool/printinfo.cpp b/src/oiiotool/printinfo.cpp
index 60e0ffc..7e6eefa 100644
--- a/src/oiiotool/printinfo.cpp
+++ b/src/oiiotool/printinfo.cpp
@@ -722,6 +722,7 @@ OiioTool::print_info (Oiiotool &ot,
         printf ("\n");
     }

+    int movie = spec.get_int_attribute ("oiio:Movie");
     if (opt.verbose && num_of_subimages != 1) {
         // info about num of subimages and their resolutions
         printf ("    %d subimages: ", num_of_subimages);
@@ -731,6 +732,8 @@ OiioTool::print_info (Oiiotool &ot,
                 printf ("%dx%dx%d ", spec.width, spec.height, spec.depth);
             else
                 printf ("%dx%d ", spec.width, spec.height);
+            if (movie)
+                break;
         }
         printf ("\n");
     }

lgritz · 2014-08-23T19:40:04Z

I really like this and want to merge it. Unfortunately, although it builds on OSX for me, it fails when I compile it on my Linux work machine. I get error messages like this:

/net/homedirs/lg/lg/proj/oiio/oiio.git/src/ffmpeg.imageio/ffmpeginput.cpp:142:9: error:
      use of undeclared identifier 'avformat_open_input'
    if (avformat_open_input (&m_format_context, file_name, NULL, NULL) !...
        ^
/net/homedirs/lg/lg/proj/oiio/oiio.git/src/ffmpeg.imageio/ffmpeginput.cpp:147:9: error:
      use of undeclared identifier 'avformat_find_stream_info'
    if (avformat_find_stream_info (m_format_context, NULL) < 0)
        ^
/net/homedirs/lg/lg/proj/oiio/oiio.git/src/ffmpeg.imageio/ffmpeginput.cpp:172:9: error:
      use of undeclared identifier 'avcodec_open2'
    if (avcodec_open2 (m_codec_context, m_codec, NULL) < 0) {
        ^
/net/homedirs/lg/lg/proj/oiio/oiio.git/src/ffmpeg.imageio/ffmpeginput.cpp:208:5: error:
      unknown type name 'AVDictionaryEntry'
    AVDictionaryEntry *tag = NULL;
    ^
/net/homedirs/lg/lg/proj/oiio/oiio.git/src/ffmpeg.imageio/ffmpeginput.cpp:255:5: error:
      use of undeclared identifier 'avformat_close_input'
    avformat_close_input (&m_format_context);
    ^

I suspect that what's happening is that it's finding ffmpeg, but it's too old. Maybe the API changed lately? On my OSX laptop, I have a modern ffmpeg 55.48.100 from Homebrew, At work on Linux, the system version of ffmpeg appears to be 52.64.2.

The best solution would be if you could use proper #if's (and version checks) to make it wok with the older releases.

But for now, I could certainly live with simply disabling the plugin if the ffmpeg version is too old to support the API calls you are using. (Yes, I could build with USE_FFMPEG=0, but I'd rather have it automatically exclude it if the versions aren't new enough. I don't want every new user to get bitten by this.)

lgritz · 2014-08-23T23:34:45Z

Another good test of robustness, I would expect this to work:

oiiotool mymovie.m4v -subimage 1000 -o frame.jpg

Crashes for me inside the FFmpegInput destructor, and I'm not quite sure why.

mikaelsundell · 2014-09-02T23:22:57Z

Did some fixes after profiling, will go through the comments above and make sure it runs as expected with oiiotool and the rest of the API.

mikaelsundell · 2014-09-03T04:30:31Z

Need input on:

OIIO_EXPORT int ffmpeg_imageio_version = OIIO_PLUGIN_VERSION;
OIIO_EXPORT ImageInput *ffmpeg_input_imageio_create () {
    return new FFmpegInput;
}
OIIO_EXPORT const char *ffmpeg_input_extensions[] = {
    "mov", NULL
};

It's possible to output all formats, extensions + decode/encode (read/write) support. The ffmpeg API needs to be initialized using av_register_all before requesting AVInputFormat/AVOutputFormat, how do we handle that with const char *ffmpeg_input_extensions[]. OIIO wants the extensions before we actually load the plugin.

virtual const char *format_name (void) const { return "movie"; }

Can be changed to output the current format_name based on the opened file but what if no file has been opened?

oiiotool mymovie.m4v -subimage 1000 -o frame.jpg

Works in latest commit. Will install an older version of ffmpeg to make sure it runs as expected, it's a few minor changes.

lgritz · 2014-09-03T05:09:00Z

Re: input_extensions[]: can we not just enumerate the usual candidates? Remember, this is just a hint for "if you see one of these extensions, try this plugin first." If an extension is not listed by any plugins, or if the one that lists it fails to open() the file, then it will try EVERY plugin until it finds one whose open() succeeds on the file. So there's no penalty on having the list incomplete for obscure formats or extensions, nor in accidentally listing one that we expect to work, but that some site's ffmpeg for quirky reasons don't support. Just list the common ones that we know about and that have a pretty good chance of working with any ffmpeg.

My comment was just illustrating that "mov" seems too narrow -- we know that mpg, avi, mp4, m4v, qt, and maybe a couple others really ought to try this plugin first.

mikaelsundell · 2014-09-03T15:15:54Z

Got it - will do that. FFmpeg is able to open exr, png, jpg so we need to make sure they don't appear as preferred extensions for the plugin as well.

Added FFmpeg hints for ffmpeg_input_extensions

mikaelsundell · 2014-09-04T04:26:36Z

Could you run this on your end on OSX and test it, I'll downgrade to FFmpeg 52.64.2 in the meantime. Once we have it running we can probably merge. Will add the writer code after that.

lgritz · 2014-10-14T03:01:18Z

Sorry, I totally dropped the ball on this one. Tomorrow I'll grab your latest and try on my end.

lgritz · 2014-10-14T18:50:15Z

This works extraordinarily well for me on OSX (especially after I modified it so change the ffmpeg log level from WARNING to FATAL -- I was getting spurious printouts to the console, something you almost never want in a library).

But I'm still having build trouble on Linux. I will investigate briefly and hopefully it's just a matter of being on different versions. If I have to, I will add proper #if's to only use ffmpeg if it's a fairly recent version. We can always come back and modify this code to support older versions later, but I don't want this review to sit around indefinitely, since even in its current state it seems useful to some people. (And I, for one, already appreciate its ability to allow oiiotool to pull single frames from animation files!)

I also think that after merging, it will need another pass to tidy up the metadata, reconciling the movie metadata names with our usual conventions (for example, "creation_date" should be "DateTime", and "title" should probably be "ImageDescription"). But again, this is something that can wait for a subsequent patch.

lgritz · 2014-10-14T19:24:21Z

Indeed, our Linux systems at work have an absurdly old ffmpeg. I don't think we should try to support it. I'll find the requisite #if to add to make it skip over if too old.

lgritz · 2014-10-14T19:38:27Z

Easy solution: have FindFFmpeg.cmake search for libavcodec/version.h rather than libavcodec/avcodec.h. The function and symbol names changed at the same old version that version.h first appeared. This will cause horribly old ffmpeg installations not to be found at build time.

Merge coming soon!

lgritz · 2014-10-14T19:51:06Z

Merged! (Squashed and a few extremely minor edits.)

mikaelsundell · 2014-10-19T10:31:48Z

Sorry for the delay, great, it's now merged! will clone master and add the writer to it.

lgritz · 2014-10-20T19:35:04Z

I'm afraid that I just had to push an update which sets USE_FFMPEG to 0 by default -- because when files were named with unknown or incorrect extensions, OIIO tries every ImageInput type to see what opens the files. FFMpeg will actually happily open many image formats, such as TIFF, and so if it is tried prior to the actual TIFF reader, the wrong reader will be used.

The ffmpeg reader needs to be modified to reject all file types that are handled by other plugins. Probably the easy thing to do is to rig it to accept true movie formats and reject all still image formats.

I was unsure how to do this, so I just turned the ffmpeg support off by default and am bouncing this back to @mikaelsundell. Mikael, after you find a fix for this, we can restore the default to compile ffmpeg if found.

See #979

sobotka · 2014-11-17T19:41:31Z

This is excellent. Just wondering how the mechanism for loading frames works in specifics? There's a bi-directional frame need that I can't see from the code if it is handled. In particular, I explain it at mpenkov/ffmpeg-tutorial#7

@mikaelsundell are you able to comment sir?

mikaelsundell · 2014-11-17T23:25:59Z

@Sabotka feel free to add your changes if I've missed something, will try to finish up the rest of the writer this week. Really stuck with to much work right now.

mikaelsundell · 2014-11-17T23:26:52Z

@sobotka sorry :-)

Mikael Sundell added 2 commits August 5, 2014 23:15

First ffmpeg commit

e0fb7fb

Removed comments

e5e6233

Fixed incorrect av_free

fac364f

Changed metadata handling, now copies all tags in dictionary Fixed incorrect reading of subframes, now reads until finished

Fixed reading of opaque frames

3061282

lgritz reviewed Aug 23, 2014
View reviewed changes

Various fixes from profiling, memory leaks and optimizations.

9a320fa

Changed format_name to FFmpeg movie

e810d8f

Added FFmpeg hints for ffmpeg_input_extensions

lgritz force-pushed the master branch from 576a9fc to f5e6edd Compare October 14, 2014 16:47

lgritz closed this Oct 14, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First ffmpeg reader #928

First ffmpeg reader #928

mikaelsundell commented Aug 5, 2014

lgritz commented Aug 5, 2014

mikaelsundell commented Aug 5, 2014

mikaelsundell commented Aug 6, 2014

lgritz Aug 23, 2014

lgritz commented Aug 23, 2014

lgritz commented Aug 23, 2014

lgritz commented Aug 23, 2014

mikaelsundell commented Sep 2, 2014

mikaelsundell commented Sep 3, 2014

lgritz commented Sep 3, 2014

mikaelsundell commented Sep 3, 2014

mikaelsundell commented Sep 4, 2014

lgritz commented Oct 14, 2014

lgritz commented Oct 14, 2014

lgritz commented Oct 14, 2014

lgritz commented Oct 14, 2014

lgritz commented Oct 14, 2014

mikaelsundell commented Oct 19, 2014

lgritz commented Oct 20, 2014

sobotka commented Nov 17, 2014

mikaelsundell commented Nov 17, 2014

mikaelsundell commented Nov 17, 2014

First ffmpeg reader #928

First ffmpeg reader #928

Conversation

mikaelsundell commented Aug 5, 2014

lgritz commented Aug 5, 2014

mikaelsundell commented Aug 5, 2014

mikaelsundell commented Aug 6, 2014

lgritz Aug 23, 2014

Choose a reason for hiding this comment

lgritz commented Aug 23, 2014

lgritz commented Aug 23, 2014

lgritz commented Aug 23, 2014

mikaelsundell commented Sep 2, 2014

mikaelsundell commented Sep 3, 2014

lgritz commented Sep 3, 2014

mikaelsundell commented Sep 3, 2014

mikaelsundell commented Sep 4, 2014

lgritz commented Oct 14, 2014

lgritz commented Oct 14, 2014

lgritz commented Oct 14, 2014

lgritz commented Oct 14, 2014

lgritz commented Oct 14, 2014

mikaelsundell commented Oct 19, 2014

lgritz commented Oct 20, 2014

sobotka commented Nov 17, 2014

mikaelsundell commented Nov 17, 2014

mikaelsundell commented Nov 17, 2014