Opened 11 years ago

Closed 11 years ago

#2672 closed defect (fixed)

Incompatible avfilter_graph_parse()

Reported by: Clément Bœsch 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 by Carl Eugen Hoyos, 11 years ago

Priority: normalimportant
Version: unspecifiedgit-master

Isn't this what HAVE_INCOMPATIBLE_FORK_ABI is for?

in reply to:  1 ; comment:2 by Clément Bœsch, 11 years ago

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 by gjdfgh, 11 years ago

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.)

Last edited 11 years ago by gjdfgh (previous) (diff)

in reply to:  3 comment:4 by Clément Bœsch, 11 years ago

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 11 years ago by Clément Bœsch (previous) (diff)

comment:5 by gjdfgh, 11 years ago

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.)

in reply to:  2 comment:6 by Carl Eugen Hoyos, 11 years ago

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 ?

in reply to:  description comment:7 by Stefano Sabatini, 11 years ago

Analyzed by developer: set
Keywords: api libav fork abi added
Reproduced by developer: set
Resolution: fixed
Status: newclosed

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.