Incorrect data structure fields on Ubuntu 14.04 with AVStream?

[emk]

I'm not sure if there's anything you can do about this one, but I thought I would mention it. If you really want to reproduce this locally, I'm happy to provide source code and step-by-step repro instructions. But mostly I'm just sharing this because:

1. Misery loves company, and
2. This might raise interesting questions about how to handle #[repr(C)] struct definitions in the FFI.

I began by adding a metadata method to Stream:

    pub fn metadata(&self) -> Option<DictionaryRef> {
        unsafe {
            let metadata = (*self.as_ptr()).metadata;
            if metadata != ptr::null_mut() {
                Some(DictionaryRef::wrap((*self.as_ptr()).metadata))
            } else {
                None
            }
        }
    }

But when I tried to iterate over stream metadata, my program crashed. So I decided it was time to try out the rr debugger, which records execution traces, and which provides support for stepping backwards in GDB. So I downloaded it, and captured a recording of the crash:

rr record target/debug/substudy tracks a.mkv

Executing up to the crash and then rewinding back out of av_dict_get revealed some funny business:

TITLE: AVATAR_AIRBENDER_BK1VOL1
Stream #0

Program received signal SIGSEGV, Segmentation fault.
0x00007fc97262131f in av_dict_get ()
   from /usr/lib/x86_64-linux-gnu/libavutil.so.52
(rr) reverse-finish 
Run back to call of #0  0x00007fc97262131f in av_dict_get ()
   from /usr/lib/x86_64-linux-gnu/libavutil.so.52

Program received signal SIGSEGV, Segmentation fault.
0x00007fc97262131f in av_dict_get ()
   from /usr/lib/x86_64-linux-gnu/libavutil.so.52
(rr) bt
#0  0x00007fc97262131f in av_dict_get ()
   from /usr/lib/x86_64-linux-gnu/libavutil.so.52
#1  0x00007fc9734ccdb0 in ffmpeg::util::dictionary::iter::Iter<'a>.Iterator::next (self=0x7fff4adc1eb0) at rust-ffmpeg/src/util/dictionary/iter.rs:32
...
(rr) reverse-next
Single stepping until exit from function av_dict_get,
which has no line number information.
ffmpeg::util::dictionary::iter::Iter<'a>.Iterator::next (self=0x7fff4adc1eb0)
    at rust-ffmpeg/src/util/dictionary/iter.rs:32
32              let entry = av_dict_get(self.ptr, empty.as_ptr(), self.cur, AV_DICT_IGNORE_SUFFIX);
(rr) p self.ptr
$1 = (enum c_void *) 0xfb000000df

Wait, what? self.ptr is 0xfb000000df? How did I get a metadata dictionary pointer that looked like 0xfb000000df? That can't be right.

So I set a hardware watchpoint on the bogus pointer, and run backwards until I see that pointer being set:

(rr) watch -l self.ptr
Hardware watchpoint 1: -location self.ptr
(rr) reverse-continue 
Continuing.
Hardware watchpoint 1: -location self.ptr

Repeating this process several times eventually brings me to:

$9 = (enum c_void *) 0xfb000000df
(rr) watch -l self.context.ptr.streams[0].metadata
Hardware watchpoint 3: -location self.context.ptr.streams[0].metadata
(rr) rc
Continuing.
Hardware watchpoint 3: -location self.context.ptr.streams[0].metadata

Old value = (enum c_void *) 0xfb000000df
New value = (enum c_void *) 0x1000000df
0x00007fc97262aef6 in av_reduce ()
   from /usr/lib/x86_64-linux-gnu/libavutil.so.52
(rr) bt
#0  0x00007fc97262aef6 in av_reduce ()
   from /usr/lib/x86_64-linux-gnu/libavutil.so.52
#1  0x00007fc9713e3baa in ?? ()
   from /usr/lib/x86_64-linux-gnu/libavformat.so.54
#2  0x00007fc97146084d in avformat_open_input ()
   from /usr/lib/x86_64-linux-gnu/libavformat.so.54
#3  0x00007fc9733a8c80 in substudy::format::input<std::path::PathBuf> (
    path=0x7fff4adc2348) at rust-ffmpeg/src/format/mod.rs:154

So the bogus pointer in self.context.ptr.streams[0].metadata is being created by av_reduce inside of avformat_open_input. But wait, av_reduce performs calculations with rational numbers. That shouldn't have anything to do with metadata. But let's take a look at AVStream again:

    pub sample_aspect_ratio: AVRational,
    pub metadata: *mut AVDictionary,
    pub avg_frame_rate: AVRational,

Yup. metadata is right between two rational fields. Time to compare the Rust structure:

#[repr(C)]
pub struct AVStream {
    pub index: c_int,
    pub id: c_int,
    pub codec: *mut AVCodecContext,
    pub priv_data: *mut c_void,
    pub pts: AVFrac, // XXX: #if FF_API_LAVF_FRAC
    pub time_base: AVRational,
    pub start_time: int64_t,
    pub duration: int64_t,
    pub nb_frames: int64_t,
    pub disposition: c_int,
    pub discard: AVDiscard,
    pub sample_aspect_ratio: AVRational,
    pub metadata: *mut AVDictionary,
    pub avg_frame_rate: AVRational,
    pub attached_pic: AVPacket,

To the C version on my local system:

typedef struct AVStream {
    int index;    /**< stream index in AVFormatContext */
    int id;
    AVCodecContext *codec;
#if FF_API_R_FRAME_RATE
    AVRational r_frame_rate;
#endif
    void *priv_data;
    struct AVFrac pts;
    AVRational time_base;
    int64_t start_time;
    int64_t duration;
    int64_t nb_frames;                 ///< number of frames in this stream if known or 0
    int disposition; /**< AV_DISPOSITION_* bit field */
    enum AVDiscard discard; ///< Selects which packets can be discarded at will and do not need to be demuxed.
    AVRational sample_aspect_ratio;
    AVDictionary *metadata;
    AVRational avg_frame_rate;
    AVPacket attached_pic;

Oooh, what's this FF_API_R_FRAME_RATE thing? Let's grep a bit:

#define FF_API_R_FRAME_RATE            (LIBAVFORMAT_VERSION_MAJOR < 55)

#define LIBAVFORMAT_VERSION_MAJOR 54

/**
 * FF_API_* defines may be placed below to indicate public API that will be
 * dropped at a future version bump. The defines themselves are not part of
 * the public API and may change, break or disappear at any time.
 */

Bingo. So now we know that ffmpeg-sys requires at least LIBAVFORMAT_VERSION_MAJOR 55 to work, but Ubuntu 14.04 only ships 54. And with 54, ffmpeg-sys will look for the metadata field where libavformat has actually stored avg_frame_rate. Which explains why backwards tracing with hardware breakpoints thinks that our metadata pointer was created by av_reduce.

pounds head on desk

I don't have any good answers here.

[meh]

Yeah, right now all I do is support the latest version, which is currently 2.8.2, it's really hard to support every version available, especially when all I'm doing is linking dynamically without touching the headers.

I did provide some features that build the latest ffmpeg and link it in statically, but there are some standing Cargo bugs that make it not work.

[emk]

Is there a recommended way to build/supply 2.8.2 on systems with an older version of ffmpeg or libav?

(Also, this was my first debugging session with rr, and I'm suitably impressed.)

[meh]

If you're on Ubuntu, you could look for some ppa that provides the latest, otherwise you can always build it from source and install it in /usr/local.

[meh]

The final ideal target would be to just pass as features static and build, and everything works magically, but I've had problems making that work, at least for running tests.

It does build it but Cargo seems to treat it as an .rlib instead of .a, so it breaks on linking.

[emk]

OK, more experimentation reveals:

• The ffmpeg project recommends this PPA for Ubuntu 14.04, which currently contains ffmpeg 2.8.1 and a set of static libraries.
• Building with that setup gives me error: could not find native static libraryavutil, perhaps an -L flag is missing?.

I used the following dependencies, both with and without static:

[dependencies.ffmpeg-sys]
version = "2.8.0"
default-features = false
features = ["static", "avcodec", "avdevice", "avfilter", "avformat",
            "swresample", "swscale"]
optional = true

[dependencies.ffmpeg]
#version = "~0.2.0"
path = "rust-ffmpeg"
default-features = false
features = ["static", "codec", "device", "filter", "format",
            "software-resampling", "software-scaling"]
optional = true

At this point, I'm not sure that there's any way to get this working on Ubuntu 14.04 LTS without asking my users to build ffmpeg from source, which I'd just as soon avoid (especially since the version number needs to be a precise match, and I don't want to build it anywhere other software might find it).

One possible solution would be for ffmpeg-sys to build its own copy of ffmpeg. Another possible solution would be to include a supplemental wrappers.c file that added tiny C wrappers for struct fields, so instead of writing stream.metadata, the code could write ffmpeg_wrapper_stream_get_metadata(stream). This might make it possible to support a wider range of ffmpeg versions, at least if you never needed to allocate any of the structs on the stack.

For my needs, the best solution may be to write a thin wrapper around the CLI tools. :-( Many of them can now output JSON, at least, and it would be easier for my users to get building.

[meh]

You don't need to define features for both ffmpeg-sys and ffmpeg, ffmpeg features define the required ones automatically, basically this.

[dependencies.ffmpeg]
optional = true
features = ["static", "build"]

That's all you should need, that will make ffmpeg-sys build ffmpeg from sources.

Can you try it out with your binary and see what happens?

[lummax]

@emk Welcome to the wonderful world that is the ffmpeg API. I had many wonderful hours wtih it :D

I actually spend my weekend getting ffmpeg to build statically and get a *.so out of my library that uses rust-ffmpeg but with static ffmpeg dependencies. In the end I triumphed. I can reduce that to a minimal project to showcase what I did (as soon as I have some time to spare the next few days).

But in the end it boils down to building ffmpeg with https://github.com/zimbatm/ffmpeg-static (with modifications) and overriding the rust-ffmpeg-sys search path. See #11.

[meh]

@lummax ideally I'd like to have ffmpeg-sys doing all that crap to build statically.

[emk]

I also need to include ffmpeg-sys directly so that I can access the loglevel options, unfortunately.

Thank you all for your help trying to get ffmpeg built! I'm probably going to go in another direction for now, but I'll probably try again later at some point.

[lummax]

@meh Me too. But there is a lot of complexity.

You want 'H264'? Get a static version of libx264!

And rust-ffmpeg-sys can not deal with every dependency. In the end the library gets too big. I think it's sensible to let the user deal with that (Especially with regards to linux distros that maybe distribute static versions of the required dependencies).

@emk Just implement the logging stuff and open a PR :D

[meh]

@emk I can work on the logging stuff if you need it, just tell me what you need safely wrapped that isn't present yet and I'll bump it on the backlog.

Me and @lummax have just been working on what we personally needed so far.

@lummax it shouldn't make it any more complex, it will just make the build.rs a little more complex, and I'm fine with it since it's all it has to do.

[meh]

@lummax to expand on that, there are already Cargo features that check what libraries you want to compile in, it's just a matter of checking for them and downloading/building them statically.

[lummax]

As promised, here you have it: https://github.com/lummax/rust-ffmpeg-static

Although I'd also like this complexity to be included in rust-ffmpeg-sys, I don't think it is the right place. The complexity (and rather long build times) are hidden, aswell as the possible tradeoffs (this linked repository disables the hardware acceleration).

I think it would be best to include an example or a showcase similar to mine in the README and let people figure this out on their own.

@saeschdivara This might also concert you as #14 touches this issue.

[meh]

They wouldn't be hidden, you'd have to opt-in with the build feature like it is already.

I'll give it a shot later.

[meh]

Heads up on this, now the build script checks headers for ifdefs, so it should work.

As in, when it's not building ffmpeg itself it will check the headers on the system to figure out the configuration.

[emk]

Nice! I ultimately ended up talking to the ffmpeg command-line tool via a batch job wrapper, which works well enough for my use cases (probing, output in various formats). But thank you for working on this! If I ever outgrow the CLI interface, I may be back. :-)