[FFmpeg-devel] [GSoC] [RFC] Abstract AVFilterFormats to AVRefCountedList for reuse

[Stefano Sabatini]

On date Thursday 2011-06-02 08:09:42 +0300, Mina Nagy Zaki encoded:
> On Thursday 02 June 2011 06:46:57 Michael Niedermayer wrote:
> > On Wed, Jun 01, 2011 at 09:32:49AM +0300, Mina Nagy wrote:
> > > ----- Original message -----
> > > 
> > > > On Mon, May 30, 2011 at 11:36:22PM +0200, Stefano Sabatini wrote:
> > > > > On date Monday 2011-05-30 12:00:01 +0300, Mina Nagy Zaki encoded:
> > > > > > AVRefCountedList will be used for lists of supported formats and
> > > > > > channel   layouts. There is a problem currently with chlayouts,
> > > > > > they are int64 while   formats are int32. I could not determine
> > > > > > why chlayouts are 64 bit, but trying   to change them will break
> > > > > > API/ABI. This means AVRefCountedList will have to   support both.
> > > > > > After discussion with Stefano I think simply having a
> > > > > > avfilter_refcounted_list_make32() will do. Video filters will have
> > > > > > to use that   since they will store PixelFormat lists, which are
> > > > > > enums and hence 32bit.
> > > > > 
> > > > > Or alternatively use int64_t and convert the enum pix_fmts[] arrays
> > > > > to int64_t arrays.
> > > > > 
> > > > > 
> > > > > Michael, Peter?
> > > > 
> > > > maybe the functions/fields could be named better but i dont have
> > > > specific ideas
> > > 
> > > Which part needs changing? The namespace (AVFilter* instead of AV*) or
> > > the 'refcounted' name itself, or perhaps 'list' ('set' is more apropos),
> > > or does it just need to be shortened (av_rclist_make, etc) ?
> > 

> > nothing needs changing its just that somehow refcounted sounds a
> > bit obscure compared to formats.
> > maybe its just the best term that exists, i dunno
> > this is not an objection to the patch 

Same for me, I suppose we can still use AVFilterFormats as well (a
channel layout can be considered a "format" as well), we can't change
it later if we don't like it.

I suppose AVRefCountedList is not very clear since it means "list of
reference counted objects", but it's not very clear what this
"reference counted objects" are, maybe AVRefCountedInts would be fine,
in alternative keeping AVFilterFormats looks fine to me (and no need
to change API).

Remains to be determined if we need to change the formats elements
from int -> int64_t for containing also channel layouts and convert
the list of enum PixelFormat in the video filters accordingly (I
suppose this can be safely done). In alternative we could templatize
the AVFilterFormats definition and have an AVFilterFormats64 (but
looks overkill to me).

I'm undecided, that's why I'm requesting other developers opinions
;-), but if I'm forced to do a choice that is:
keep the AVFilterFormats name and extend it to contain int64_t *formats.

> It makes a little more sense when you see it in usage as a set of 
> formats/layouts... but I'm actually starting to doubt the whole idea. Are 
> there situations/filters that will actually benefit from layout
> negotiation.

> The one example I have (af_earwax) actually *doesn't* benefit, it
> makes more sense for it to simply abort when faced with the wrong
> channel layout.
> 

> Can someone comment on whether or not layout negotiation is going to be useful 
> or have I over-engineered this?

I suppose layout negotiation is useful, indeed one of the high-level
design objectives of lavfi is to make it as automatic as possible, if
the framework can automatically insert an aconvert filter for changing
layout, that's better than requesting the user to insert it by hand.

On the other hand I really can't still understand the problem with
earwax and why it's converting it back from stereo->mono, maybe you
can illustrate the problem (and I'll try it myself tomorrow).
-- 
FFmpeg = Faithless & Fabulous Merciful Philosophical Enigmatic Gymnast