[FFmpeg-soc] libavfilter review

[Bobby Bingham]

On Sun, 04 Nov 2007 21:33:10 +0100
Vitor <vitor1001 at gmail.com> wrote:

> Hi, and sorry to resurrect an old thread.
> 
> Michael Niedermayer wrote:
> > Hi
> > 
> > On Sun, Aug 19, 2007 at 12:36:15PM -0400, Bobby Bingham wrote:
> >> On Sat, 18 Aug 2007 19:22:38 +0200 Michael Niedermayer wrote:
> >>> On Sat, Aug 18, 2007 at 12:12:08PM -0400, Bobby Bingham wrote:
> >>>> On Fri, 17 Aug 2007 21:10:15 +0200 Michael Niedermayer wrote:
> >>>>
> >>>>> the query_formats system is very flexible, it takes a
> >>>>> AVFilterLink so a filter could have a fixed list of supported
> >>>>> input pix_fmts and make the output query_formats depend on the
> >>>>> input pix_fmt or the other was around well i dont think the
> >>>>> other way around would work but how should the user know that?
> >>>>> and whats the sense of this overly flexible system if it doesnt
> >>>>> work with anything but the obvious simple supported output
> >>>>> pix_fmt depends upon input formats, it would be alot clearer if
> >>>>> query_formats would take a list/array of input pix_fmts as
> >>>>> argument (or a array or pix_fmt, flags pairs) where the flags
> >>>>> could indicate if the pix_fmt can be provided without colorspace
> >>>>> conversation, but maybe thats not really usefull and a simpler
> >>>>> prefer first possible format in the list system would work
> >>>>> equally well
> >>>>>
> >>>>>
> >>>>> also what happens in the following case:
> >>>>> src -> filter -> destination
> >>>>>
> >>>>> src supports formats A and C
> >>>>> destination supports formats B anc C
> >>>>> and the filter supports A,B,C inputs and output=input
> >>>>>
> >>>>> if i understood the code correctly this would fail
> >>>>>
> >>>> I think I've got an idea which will be less absurdly flexible,
> >>>> and will support graphs like your example without requiring
> >>>> conversion.  The only thing is that I might need to set the
> >>>> restriction that all the inputs of a filter must always be
> >>>> operating on the same colorspace, and similar for outputs.
> >>>>
> >>>> This of course doesn't affect all those filters with only simple
> >>>> inputs and outputs.  And I expect most filter authors would only
> >>>> support this case anyway.  But before I start coding it, I want
> >>>> to check if such a restriction would be acceptable.
> >>> its probably ok (i cant think of a case where a filter would want
> >>> 2 inputs with differing colorspace)
> >>>
> >> Unfortunately, I thought of a case.  The filtergraph filter, being
> >> simply a composite of various subfilters, may legitimately have
> >> inputs with multiple colorspaces.  And doing so may optimize the
> >> number of conversions.
> >>
> >> I'm afraid the more I look at the problem, the more I realize it's
> >> not something I can come up with a good solution to by Monday.  I
> >> think I'll add support for autoconversion to the current system so
> >> that it at least works in all cases, and I'll have to work on a
> >> better system after the end of summer of code, because there are
> >> enough other areas I can actually make improvements on before the
> >> deadline if I don't get hung up on this one thing.
> >>
> >> Of course, I'm open to any ideas anybody has regarding a solution
> >> to colorspace negotiation...
> > 
> > ive thought about cs negotiation a little and ive come up with the
> > following possible solution
> > 
> > each filter link has 2 IDs (integers or  pointers or something)
> > one corresponding to the input into the link and one corresponding
> > to the output (alternatively we could have always 2 links with a
> > dummy filter between)
> > 
> > now if 2 IDs anywhere within the filter graph are equal that means
> > that the colorspace must be equal and each ID has a list of valid
> > colorspaces
> > 
> > now to solve it you just merge the 2 IDs on each link
> > if both the corresponding lists have a common colorspace then merge
> > is possible if not conversation is needed
> > 
> > example: 
> > the case from above:
> >>>>> src -> filter -> destination
> >>>>>
> >>>>> src supports formats A and C
> >>>>> destination supports formats B anc C
> >>>>> and the filter supports A,B,C inputs and output=input
> > 
> > src b-->c filter c-->d dst
> > 
> > b={A,C}
> > c={A,B,C}
> > d={B,C}
> > 
> > first we merger b and c leading to
> > 
> > src b-->b filter b-->d dst
> > 
> > b={A,C}
> > d={B,C}
> > 
> > and next: b and d
> > 
> > src b-->b filter b-->b dst
> > 
> > b={C}
> > 
> 
> That will work if the filters have either input_fmt=output_fmt or
> fixed output format. But for the case of, for example, a gamma filter
> which would turn RGB24 into RGB32 and BGR24 into BGR32 it won't work.
> In schema:
> 
> src --> filter --> dst
> 
> src supports RGB24 and BGR24
> dest supports BGR32
> filter outputs BGR32 if input is BGR24 and RGB32 if input is RGB24
> 
> The solution without conversion is
> 
> src BGR24 -> BGR24 filter BGR32 -> BGR32 dest
> 
> -Vitor

As far as I'm aware, RGB24 and RGB32 only differ in that RGB32 has an
unused byte for each pixel, where an alpha value would go for RGBA.  So
unless I miss something, I don't see why a filter would do that exactly.

But it still raises a good point.  When I had been brainstorming ideas
for colorspace negotiation, I tried taking such cases into account, and
it just made the whole thing so much more complicated.  After seeing
Michael's suggestion, I figured that in almost all cases, a filter will
either have output format = input format, or will be some sort of
colorspace conversion filter where the output and input formats don't
need to be related.

How common is this case where the possible output formats depend on the
input format, but do not equal it, really?  If it's rather uncommon
like I suspect, I'd rather have those filters deal with it themselves
than introduce all the extra complexity into libavfilter.  For example,
in your example above, the filter could without too much trouble also
have a routine to perform the operation on RGB24 -> BGR32.  It's just
two bytes swapped from what it would normally do.

--
Bobby, who finally found time to implement Michael's suggestion
today ...