Opened 3 years ago

Closed 3 years ago

#2672 closed defect (fixed)

Incompatible avfilter_graph_parse()

Reported by: ubitux Owned by:
Priority: important Component: avfilter
Version: git-master Keywords: api libav fork abi
Cc: Blocked By:
Blocking: Reproduced by developer: yes
Analyzed by developer: yes

Description

In 6119b23a, API was broken (on purpose):

 int avfilter_graph_parse(AVFilterGraph *graph, const char *filters,
-                         AVFilterInOut *inputs, AVFilterInOut *outputs,
+                         AVFilterInOut **inputs, AVFilterInOut **outputs,
                          void *log_ctx);

While that API break was not really a problem at that time, the Libav fork already existed and didn't pick that change. This causes an API incompatibility between the two projects, which needs to be fixed somehow.

AFAICT there is unfortunately no clean way to do that. Best we can do seems to be to restore the prototype automatically at next major bump (but that might break a lot of apps). Note that some applications are already using this new prototypes.

Change History (7)

comment:1 follow-up: Changed 3 years ago by cehoyos

  • Priority changed from normal to important
  • Version changed from unspecified to git-master

Isn't this what HAVE_INCOMPATIBLE_FORK_ABI is for?

comment:2 in reply to: ↑ 1 ; follow-up: Changed 3 years ago by ubitux

Replying to cehoyos:

Isn't this what HAVE_INCOMPATIBLE_FORK_ABI is for?

That would require applications to define it themselves before including avfilter.h, and make sure the build is also an incompatible fork ABI build; which is basically more pain for them than just doing ifdefery between the two versions of the function themselves.

comment:3 follow-up: Changed 3 years ago by gjdfgh

In a perfect world you'd probably ask the Libav devs to add a (redundant) new symbol, which would behave the same with two libraries. But I bet $10 that we can't have this. (Maybe you can bait them It's probably best to change the API to Libav's and let the ffmpeg exclusive users burn for it. (Personally I'm already having a ifdef mess in my application to deal with this; it would of course become worse.)

Then the mess wouldn't exist in future ffmpeg releases, at least.

In a perfect world you'd probably ask the Libav devs to add a (redundant) new symbol, which would behave the same with two libraries. But I bet $10 that we can't have this. (Maybe you can bait them by introducing a new utility function, that merges avfilter_graph_config()? (Or something similar. I don't understand why these are separate.)

Version 0, edited 3 years ago by gjdfgh (next)

comment:4 in reply to: ↑ 3 Changed 3 years ago by ubitux

Replying to gjdfgh:

It's probably best to change the API to Libav's and let the ffmpeg exclusive users burn for it. (Personally I'm already having a ifdef mess in my application to deal with this; it would of course become worse.)

Then the mess wouldn't exist in future ffmpeg releases, at least.

Except mpv which you maintain, there is XMBC IIRC. We might want to send them a patch for this.

In a perfect world you'd probably ask the Libav devs to add a (redundant) new symbol, which would behave the same with two libraries. But I bet $10 that we can't have this. (Maybe you can bait them by introducing a new utility function, that merges avfilter_graph_config()? Or something similar. I don't understand why these are separate.)

That means they would need to break their API the same way we did, which is not reasonable, so they won't agree (and they would be right, since lavfi API is not as unstable as it was, it would need a proper bump and function rename).

Last edited 3 years ago by ubitux (previous) (diff)

comment:5 Changed 3 years ago by gjdfgh

That means they would need to break their API the same way we did, which is not reasonable, so they won't agree

No, I was suggesting to introduce an alternative function, which does the same thing. Like avfilter_graph_parse2(), which would have the same signature on both projects.

Or plan B: introduce a function that is useful on its own, such as introducing a function that merges avfilter_graph_parse() and avfilter_graph_config(). (I don't know if they can be merged, it was just the first thing that caught my eye when I looked for something that seemed redundant.)

comment:6 in reply to: ↑ 2 Changed 3 years ago by cehoyos

Replying to ubitux:

Replying to cehoyos:

Isn't this what HAVE_INCOMPATIBLE_FORK_ABI is for?

That would require applications to define it themselves before including avfilter.h

How is that different from the other places using it?
The point is that packagers define it to produce an ABI-compatible library, applications will never use it.

and make sure the build is also an incompatible fork ABI build; which is basically more pain for them than just doing ifdefery between the two versions of the function themselves.

How would the ifdefery work if not through HAVE_INCOMPATIBLE_FORK_ABI ?

comment:7 in reply to: ↑ description Changed 3 years ago by saste

  • Analyzed by developer set
  • Keywords api libav fork abi added
  • Reproduced by developer set
  • Resolution set to fixed
  • Status changed from new to closed

Replying to ubitux:

In 6119b23a, API was broken (on purpose):

 int avfilter_graph_parse(AVFilterGraph *graph, const char *filters,
-                         AVFilterInOut *inputs, AVFilterInOut *outputs,
+                         AVFilterInOut **inputs, AVFilterInOut **outputs,
                          void *log_ctx);

While that API break was not really a problem at that time, the Libav fork already existed and didn't pick that change. This causes an API incompatibility between the two projects, which needs to be fixed somehow.

AFAICT there is unfortunately no clean way to do that. Best we can do seems to be to restore the prototype automatically at next major bump (but that might break a lot of apps). Note that some applications are already using this new prototypes.

Should be fixed in:

commit 838bd731393a29a41a86ff15ccf972f967306319
Author: Stefano Sabatini <stefasab@gmail.com>
Date:   Tue Jul 2 01:39:14 2013 +0200

    lavfi: create Libav-API compatibility layer for avfilter_graph_parse() at the next bump
    
    Add function avfilter_graph_parse_ptr() and favor it in place of
    avfilter_graph_parse(), which will be restored with the old/Libav
    signature at the next bump.
    
    If HAVE_INCOMPATIBLE_LIBAV_API is enabled it will use the
    Libav-compatible signature for avfilter_graph_parse().
    
    At the next major bump the current implementation of
    avfilter_graph_parse() should be dropped in favor of the Libav/old
    implementation.
    
    Should address trac ticket #2672.
Note: See TracTickets for help on using tickets.