Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SIGSEGV iterating stream #64

Open
scottlamb opened this issue Jul 18, 2016 · 12 comments
Open

SIGSEGV iterating stream #64

scottlamb opened this issue Jul 18, 2016 · 12 comments

Comments

@scottlamb
Copy link

scottlamb commented Jul 18, 2016

I'm probably doing something dumb, but even so I think a SIGSEGV in safe code is supposed to be impossible.

#[macro_use] extern crate ffmpeg;

use ffmpeg::format;
use ffmpeg::format::network;
use std::env;

fn main() {
    let url = env::args().nth(1).expect("missing url");
    ffmpeg::init().unwrap();
    network::init();
    let open_options = dict![
        "rtsp_transport" => "tcp",
        "probesize" => "262144",
        "user-agent" => "rtsptest",
        "stimeout" => "10000000"
    ];
    let mut input = format::input_with(&url, open_options).unwrap();
    println!("open");
    for (stream, mut pkt) in input.packets() {
        println!("packet");
    }
}

When run with some URL (for example, rtsp://wowzaec2demo.streamlock.net/vod/mp4:BigBuckBunny_115k.mov), it crashes with a segfault.

GDB says this:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  ffmpeg::format::context::input::{{impl}}::next (
    self=<error reading variable: Cannot access memory at address 0xfffffffffffffff0>)
    at /home/slamb/.cargo/registry/src/github.com-1ecc6299db9ec823/ffmpeg-0.2.0-alpha.1/src/format/context/input.rs:164
164                             match packet.read(self.context) {

which I guess means that the location where the self pointer is stored is invalid? Not sure why that would be but I'll poke at it a bit.

I tried a couple versions of rust in case it's a problem with the compiler/runtime:

rustc 1.10.0-nightly (476fe6eef 2016-05-21)
rustc 1.12.0-nightly (34f35ed29 2016-07-17)
@meh
Copy link
Owner

meh commented Jul 18, 2016

What version of ffmpeg do you have installed on the system?

This crate is still on ffmpeg-2.8, still trying to figure out the best way to support multiple versions of ffmpeg, what's happening looks like a stack overflow because some struct definitions are different in 3.0 from 2.8.

If that's not the case I'll look into it, segfaults definitely shouldn't happen.

@scottlamb
Copy link
Author

Thanks!

I was thinking from a quick glance at rust-ffmpeg-sys/build.rs that it always downloaded a ffmpeg tarball during its compilation, rather than relying on a system package. But in hindsight, the compilation was too fast for that...it does that only with the feature build. (Which doesn't quite seem to work out of the box because it's pulling in ffmpeg-sys version 2.8.8, and there's no matching tarball on ffmpeg.org. So haven't tried it quite yet.)

Okay, so if I understand correctly Rust FFI code has to duplicate the definition of structs used across the interface boundary and basically hope that they're in sync with the canonical C version? Hmm, that sucks.

And in my particular case, yes, I have a funny version installed on my system. It's some git version (it says "ffmpeg version N-77403-g70f13ab") that's probably halfway between 2.8 and 3.0.

A few ideas:

  • It seems kind of like the challenges of binary compatibility with dynamically linked libraries. A lot of C libraries deal with this by using dynamic allocation of those structs + accessor functions so they don't have to expose/freeze their struct definitions. But ffmpeg doesn't seem to provide such accessors for everything. D'oh.
  • Maybe generate these bindings automatically? I think https://github.com/kali/opencv-rust is doing that; the README.md mentions hdr_parser.py and gen_rust.py.
  • Or just never trust the system-installed one? always ship one or at least verify that the version is as expected?

@meh
Copy link
Owner

meh commented Jul 18, 2016

Ah yeah, I had to stop using the pre-release style I was using and I forgot to fix the build.rs for that, in the future the hope was to just build ffmpeg and hope it works correctly, but when I worked on that part there were issues caused by cargo, maybe that's fixed now.

About using a generator, it's not that easy sadly, ffmpeg has a gazillion configure flags that may or may not change the shape of structs and presence of functions, so generating the whole bindings automatically would be even more painful.

There are some checks based on the headers to know what fields should be, but that's as far as it goes.

I'll fix the build.rs now so you can try doing that.

@meh
Copy link
Owner

meh commented Jul 18, 2016

Alright the build.rs should be fixed, make sure to pass the right features to the ffmpeg crate, meaning build and the other build-* options.

@ReactorScram
Copy link

I have the exact same segfault on the same line, using ffmpeg 3.0 from the Arch repos.

@meh
Copy link
Owner

meh commented Jul 22, 2016

Then it's definitely 3.0, I haven't had the time to update the changes between 2.8 and 3.0, which means it's a struct having differently sized fields and corrupting the stack.

@scottlamb
Copy link
Author

In my project, I just switched to using a C wrapper around ffmpeg that provides a more stable API, and then using that from Rust rather than accessing members of ffmpeg's giant structs directly. I wrapped a very small part of ffmpeg and without as nice an API as your crate, but it's enough to see what the approach looks like.

scottlamb/moonfire-nvr@857a66f

@meh
Copy link
Owner

meh commented Sep 21, 2017

@scottlamb have you had a chance to try with the latest stuff? @retrry worked on using bindgen to generate stuff and that should have at least made things safer wrt ABI.

@scottlamb
Copy link
Author

Sigh. No. One would think that I would have looked at the latest status of the project before implementing something myself but I'd been using an old fork for a while (before versions that stopped working with ffmpeg 2.x, iirc) and was only watching this project through this bug.

I suppose one upside of what I did is that it works without having to have bindgen installed, which I remember is a bit of a pain on older platforms.

@meh
Copy link
Owner

meh commented Sep 21, 2017

@scottlamb from what I know you don't really need to have anything installed for bindgen to work now, it's just a library the build script can use like any other.

@retrry
Copy link
Contributor

retrry commented Sep 21, 2017

@meh you need to have clang installed for bindgen.

@meh
Copy link
Owner

meh commented Sep 21, 2017

@retrry ah, forgot about that, I just assumed everyone has that 🐼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants