PyAV icon indicating copy to clipboard operation
PyAV copied to clipboard

OutputContainer: Open output with avio_open2 and pass "protocol_options"

Open rawler opened this issue 4 years ago • 6 comments

Allows overriding protocol-specific options such as HTTP method and HTTP retrial policy

rawler avatar Sep 07 '20 19:09 rawler

Do note that this introduces a non-backward-compatible change in open by adding protocol_options in the middle of the argument-list. Accidents is mitigated by ensuring that any of the _options parameters must be given by name.

The alternative would be to add protocol_options to the end, but it would look a bit messy.

rawler avatar Sep 09 '20 13:09 rawler

Sorry, typo and sorry for long response-time. Updated the docstring and re-reviewed the PR myself. I think it's in order now.

rawler avatar Jul 26 '22 15:07 rawler

HI @rawler! I have to admit I'm quite confused by the handling of "options" (even before your changes). We're asking the user for distinct options / container_options / protocol_options, then we're merging all these together. How come the new "protocol_options" cannot be passed via the existing "container_options"?

jlaine avatar Jul 28 '22 07:07 jlaine

I definitely agree r.e. container vs. protocol-options (and global options). At some point, I think it needs to be cleaned up.

Container-options could not be used to pass protocol-options, since it was not passed to avio_open in OutputContainer. When making the PR, I considered just passing container-options to avio_open, but it felt wrong, solidifying the behavior of mixing all the options up. On the InputContainer-side however, we use avformat_open_input, which takes both protocol and container-options mixed, so enforcing separated options would be a breaking change. With this API, it's at least possible to write application-code that cleanly separates the types of options, (but also still possible to confuse them).

Another option could of course be to merge options back into a single parameter in the API. I think ffmpeg option names typically does not collide between protocol/container/codec.

rawler avatar Jul 28 '22 07:07 rawler

I have to admit I'm still confused, you are passing the "protocol_options" to:

  • avio_open2
  • avformat_write_header
  • avformat_open_input

Which ones actually use them? I'm not convinced that we actually want to add protocol_options, it seems to be treated 100% like container_options, so I can't really tell what we gain in exchange for the breaking API change.

jlaine avatar Jul 28 '22 12:07 jlaine

Sorry again for even slower response-time. Let me try it from a different angle.

What I really need is a way to pass options to avio_open2. Note that this is already done internally by avformat_open_input, so can already be done for input-containers, but I need a way to do it for output as well.

I can see a few different ways to achieve that;

  • Pass self.container_options. That somehow seems wrong. avio_open2 does not really deal with containers at all, just the underlying I/O-protocols.
  • Pass self.options to avio_open2. This would create different API:s where container_options reaches avio_open2 for InputContainer, but not for OutputContainer.
  • Pass self.options | self.container_options. Same problem as first, and unclear to me why we even have different option-structs if always we end up passing the union regardless?
  • Pass self.protocol_options alone to avio_open2. This would work, but again create an inconsistent API between InputContainer and OutputContainer where both would support practically the same arguments, but apply them differently.

At this point, I assume the reason we're passing self.options | self.container_options for InputContainer is basic case of backwards compatibility, while enabling/encouraging applications to start passing options separately? I'm guessing we'll at some point want to break that, but make it possible for applications to be rewritten first? (Otherwise, I've misunderstood the intention with container_options.)

Given the above assumption what this PR is trying to do is; Add separate options for the 3d layer of the stack (Codecs, Containers, I/O Protocols), while not breaking backwards compatibility, and ensuring that the API remains consistent across InputContainer and OutputContainer.

  • Given that avformat_open_input already have passed self.options | self.container_options to avio_open2, we must keep doing that, or risk breaking stuff.
  • Given that we want to start separating protocol_options, we now allow applications to pass it in separately, and can (at some later point) deprecate it being passed to the "wrong" layer.

On the flip-side; if we don't want to enable applications to pass protocol-options separately and explicitly, why do we want it for container-options?

rawler avatar Nov 18 '22 17:11 rawler

Too old.

WyattBlue avatar Mar 05 '24 05:03 WyattBlue