meh / rust-ffmpeg

Safe FFmpeg wrapper.
Do What The F*ck You Want To Public License
460 stars 96 forks source link

format::open_with consumes options dictionary #63

Open scottlamb opened 8 years ago

scottlamb commented 8 years ago

I'm trying to port over some C++ code that opens an input with options and then ensures all the options were understood:

    int ret = avformat_open_input(&stream->ctx_, source.c_str(), nullptr, dict);
    if (ret != 0) {
      *error_message = AvError2Str("avformat_open_input", ret);
      return std::unique_ptr<InputVideoPacketStream>();
    }

    if (av_dict_count(*dict) != 0) {
      std::vector<std::string> ignored;
      AVDictionaryEntry *ent = nullptr;
      while ((ent = av_dict_get(*dict, "", ent, AV_DICT_IGNORE_SUFFIX)) !=
             nullptr) {
        ignored.push_back(StrCat(ent->key, "=", ent->value));
      }
      LOG(WARNING) << "avformat_open_input ignored " << ignored.size()
                   << " options: " << Join(ignored, ", ");
    }

See also the doxygen for avformat_open_input which describes the options parameter as follows:

A dictionary filled with AVFormatContext and demuxer-private options. On return this parameter will be destroyed and replaced with a dict containing options that were not found. May be NULL.

Unfortunately format::open_with consumes the supplied dictionary without returning the new one:

pub fn input_with<P: AsRef<Path>>(path: &P, options: Dictionary) -> Result<context::Input, Error> { ... }

so as far as I can tell, it's not possible to duplicate this functionality.

In my case, I'd like to warn if the stimeout option is ignored, as I've seen with libav and older versions of ffmpeg. I'd imagine there are other options in a similar situation; only supported by some versions of ffmpeg or under some circumstances.

meh commented 8 years ago

I guess a workable way would be to pass the new Dictionary to the context::Input, so you can access it.

I have to pass it by value instead of passing a DictionaryMut because it changes the actual pointer instead of doing something sane.

Would that work for your use case?

meh commented 8 years ago

Actually that wouldn't make much sense, it could pass back a tuple, but that would make the API a little uglier.

Any ideas?

scottlamb commented 8 years ago

Not sure.

The closest match to the underlying API would be forinput_with to take a &mut Dictionary and modify the given Dictionary so that it points to the new AVDictionary afterward. But I'm assuming you're wanting to avoid inout parameters. And it does look like the argument afterward is logically a separate dictionary—in particular, even if the original was created with AV_DICT_DONT_STRDUP_KEY|AV_DICT_DONT_STRDUP_VAL, the new dictionary's entries are strdup()ed, so they don't point to the same place or have the same lifetime.

What's the concern with passing the new dictionary to the context::Input? That sounds pretty good to me in terms of making it available when necessary but not complicating usage or introducing backward compatibility problems for people who don't care. But I'm new to Rust (and your library, of course) so you must be seeing a problem I'm not.

meh commented 8 years ago

I wouldn't mind an inout parameter for that, the problem is, as you said, ffmpeg effectively takes ownership of the Dictionary you give it, and gives you a new one with the unused key/value pairs.

The concern with passing it to the created context is that semantically speaking it's unrelated to the context itself, but to the opening operation so I should find a way to use a DictionaryMut so the behavior will be identical to ffmpeg.

So yeah, it's mostly a technical problem.