bestsource icon indicating copy to clipboard operation
bestsource copied to clipboard

Pass custom options to libavformat/libavcodec

Open animetosho opened this issue 1 year ago • 4 comments

This is an incomplete PR which adds three parameters to VideoSource, enabling the ability for users to pass in custom options to libavformat and libavcodec.
The new parameters are:

  • format_opts: adds to the existing LAVFOptions map which is passed onto avformat_open_input
  • stream_opts: passed onto avformat_find_stream_info for each stream
  • codec_opts: passed onto avcodec_open2

Since VapourSynth doesn't support a VSMap in a VSMap, the parameters must be passed as an array of string pairs:

# like this:
format_opts = ["key1", "value1", "key2", "value2"]
# instead of what one might expect...
format_opts = {"key1": "value1", "key2": "value2"}

I'm mostly seeking feedback at this stage, so the following aren't done yet:

  • Only implemented for VideoSource, missing for AudioSource
  • Not implemented for AviSynthPlus (might not be feasible?)
  • stream_opts and codec_opts are not written to the cache file (or checked on load)
  • README not updated

If there's interest in this change, I can look at implementing the above.

animetosho avatar Sep 10 '24 06:09 animetosho

Which options that I didn't expose are actually useful? You can probably implement it (poorly) in avs+ with the ... argument stuff

myrsloik avatar Sep 10 '24 08:09 myrsloik

Which options that I didn't expose are actually useful?

For most use cases, I'd imagine none. The options are mostly for more niche use cases.
Libav implemented the available options, so they clearly have a use somewhere (or at least the developer thought there would be a use case).

For me, I'm mostly interested in libavcodec's x264_build option to cater to non-spec compliant encodes created by older x264 builds.
I've tried to enable this by catering to a wider range of use cases rather than my specific need, whilst keeping the enabling code relatively small.

You can probably implement it (poorly) in avs+ with the ... argument stuff

Thanks for the suggestion - it's probably not worth it there if that's the case.

animetosho avatar Sep 10 '24 09:09 animetosho

Not sure if I think it's worth exposing in this way. I'll probably add codec options to the LW*Decoder classes just for the sake of completion though so your implementation will be simpler. Btw, why 3 sets of options? Can you give a single example of when stream opts are actually useful?

myrsloik avatar Sep 10 '24 13:09 myrsloik

Thanks for the feedback!

If you'd rather the parameter(s) be passed via a LWVideoDecoder property instead of the constructor, I can change that for you (should the BestVideoSource class also be changed?).

Since this added the framework for user-supplied options, I figured it was more "why not three options?". If you disagree, I can remove some of them.

Or if you have your own idea on how to approach this, I can close this PR.

On a different note, I'm not sure if you got notified of it, but I also submitted a PR to vs-imwri, which I'm interested in feedback for.

animetosho avatar Sep 10 '24 18:09 animetosho

I take it there's no interest in this PR (feel free to reopen otherwise).

I'll point out that when writing this PR, I noticed that a dictionary isn't freed when this exception is thrown - it might be worth fixing that.

animetosho avatar Sep 21 '24 05:09 animetosho