FFmpeg reader should not accept still image files

[lgritz]

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.

[mikaelsundell]

Will have a look at this tonight!

[sobotka]

So I've been looking into this a bit, and hope to implement this soon. For future reference however, I thought I'd post a likely path to make this work well.

The general method to initialize FFMPEG is to rely on av_register_all, as is done in the FFMPEG input code:
https://github.com/OpenImageIO/oiio/blob/43f4f614558eb3b17ab3bc0de2a8836de9878db8/src/ffmpeg.imageio/ffmpeginput.cpp#L144

This can instead be initialized per-format using av_register_input_format, which can be cherry picked from the list via allformats.c http://ffmpeg.org/doxygen/trunk/allformats_8c_source.html

[lgritz]

Is there an easy single call that will register all the movie formats, but none of the still image formats? That's more or less the behavior we want. Or is our only choice to replicate the whole register_all() and just exclude what we don't want?

I suppose another approach is to bypass this entirely, and just check the file extension ourselves and fail (before even trying ffmpeg) for all but the named movie formats we wish to support. Usually, we will let our plugins try opening any file regardless of extension, but maybe the expedient way to handle this is, for this one plugin, to actually not attempt to open files that don't by name appear to be the right type.

[lgritz]

Incidentally, master has really accumulated a lot of stuff over the last few months, and it would be nice to cut a release branch. I'd like to aim for close to the beginning of December to do a freeze/branch. I would really like 1.5 to be able to enable the movie formats by default, which requires one of these solutions to be implemented. If nobody is able to provide a patch by that time that does something smart, I may just do the easy thing I described above of rejecting open attempts when the filename doesn't have one of the approved extensions, at least as a temporary measure.

[wrosecrans]

Would it make any sense to tinker with the way that OIIO tries the plugins
for formats, and allow the plugin to have some sort of "priority" or
"confidence," such that the ffmpeg reader could still attempt to load PNG
files or whatever, but it could notify the library that if anything else
will read it, the ffmpeg reader isn't very confident that it is the right
choice. But, if no other reader is available then ffmpeg will perhaps be
better than just giving up if it is indeed a format that ffmpeg can read.
Or the ffmpeg reader could set itself with below normal priority so that
more specific plugins with high or normal priority are always checked first.

On Tue, Nov 18, 2014 at 11:48 PM, Larry Gritz notifications@github.com
wrote:

Is there an easy single call that will register all the movie formats, but
none of the still image formats? That's more or less the behavior we want.
Or is our only choice to replicate the whole register_all() and just
exclude what we don't want?

I suppose another approach is to bypass this entirely, and just check the
file extension ourselves and fail (before even trying ffmpeg) for all but
the named movie formats we wish to support. Usually, we will let our
plugins try opening any file regardless of extension, but maybe the
expedient way to handle this is, for this one plugin, to actually not
attempt to open files that don't by name appear to be the right type.

—
Reply to this email directly or view it on GitHub
#979 (comment).

[lgritz]

Right, there is a general problem of "how do we deal with multiple plugins that are able to read a particular file format?"

Until this plugin, I think that all of the major formats could only be opened by one reader. As a second line of defense, since it will always FIRST try using the reader whose input_extensions have an exact match (i.e., for foo.tif, it will alway try tiff.imageio before any other plugins), we are careful that the lists of format extensions are mutually exclusive. (Aside: we don't have a mechanism to guarantee this. But it's not clear that it comes up enough in practice to care.)

The problem really only exists for a file that is oddly named (e.g., a TIFF file named "foo.zif") or incorrectly named ("foo.exr" that's really a TIFF file). If no matching extension is found (or the reader matching the extension fails the open), it will cycle through all readers, in some potentially arbitrary order, until it finds one that opens the file. And in this case, the ffmpeg one succeeds first, coincidentally (possibly because "ffmpeg" is alphabetically before "tiff").

I can see three possible solutions:

• For underlying format libraries that are "over-zealous" in what they will read, we can artificially constrain them to fail for files that we know should rightfully be handled by other plugins. This is my proposed short-term solution, and perhaps is an adequate long-term solution considering that we've only had one problematic case after 6 years of heavy development and use (and, as far as I know, only in a contrived situation designed to test what happens with misnamed files).
• Establish some kind of priority order of plugins to try after the plugin associated with the extension fails. That is, when we throw up our hands and try every plugin, maybe the most important formats (tiff, openexr, etc.) should always go before something like ffmpeg. This would unambiguously solve the problem, though my intuition is that defining a single canonical order will be contentious and error-prone.
• Ignore the problem completely, and just say that if the file doesn't use a known extension or is misnamed with the wrong extension, you get what you deserve: some reader may succeed, but it's hard to know if it's the "best" reader for that format. I can fix my "wrong name" test case somehow to expect different behavior.

[sobotka]

That's more or less the behavior we want. Or is our only choice to replicate the whole register_all() and just exclude what we don't want?

The av_register_all() function is nothing more than a series of inlined individual registers.

I would think that it makes more sense to gradually add (and test) various formats that align with the file extensions given that FFMPEG also handles audio formats beyond the already discovered still images. This would probably wreak havoc in some edge cases, no?

[lgritz]

I believe this was fixed by #1010 and we never went back and closed this ticket. Please re-open or re-submit if I'm wrong.