[FFmpeg-devel] [libav-devel] [ffmpeg-devel] [PATCH 1/2] lavfi: make AVFilteFormats use int64_t lists to support channel layouts.

[Stefano Sabatini]

On date Sunday 2011-06-12 14:42:17 +0300, Mina Nagy Zaki encoded:
> On Thursday 09 June 2011 17:36:16 Stefano Sabatini wrote:
> > On date Thursday 2011-06-09 13:25:25 +0300, Mina Nagy Zaki encoded:
> > > The list type was changed to int64_t to be able to hold
> > > channel layouts.
> > > 
> > > Usage of avfilter_make_format_list for PixelFromats/[AV]SampleFormats
> > > had to be changed to use int64_t[] instead of enums, as they are 32bit.
> > 
> > I discussed this with Mina and this looked like the best solution for
> > avoiding separate int/int64_t functions. If you have reasons to think
> > there are better solutions, please comment.
> > 
> [...]
> 
> Ok, I just realized there's a *third* option. Keep all the 32 bit ints and 
> drop support for any channel layouts that have more than 32 channels. I was 
> told that it was originally int64 to support formats that can do more than 32 
> channels. Right now though, the only value defined that's outside the int32 
> range is the value for AV_CH_LAYOUT_NATIVE which is unapplicable to channel 
> layout negotiation anyway.
> 
> This way, no breaking ABI/API, no losing type-safety. It's just that filters 
> will not support more than 32 channels. FFmpeg itself doesn't support it, and 
> int64 is only there for *possibly* supporting it. So I don't see what's wrong. 
> If we ever support it in FFmpeg, we can make the int64 change in lavfi then. 

So resuming:

1) make AVFilterFormats contain int64_t, and implement two separate:

AVFilterFormats *avfilter_make_format_list  (const int     *fmts)
AVFilterFormats *avfilter_make_format64_list(const int64_t *fmts)

Type-safe, but requires some code duplication (source code duplication
would be avoided by using a macro DEFINE_AVFILTER_MAKE_FORMAT_LIST(type)).

2) always use avfilter_make_format_list(const int64_t *fmts)

in this case we don't need two distinct avfilter_make_format*_list
functions, but we lose type-safety/debuggability and we need to update
current filters enum PixelFormat pix_fmts lists.

3) always use avfilter_make_format_list(const int *fmts)

but we can't support non-int and AV_CH_LAYOUT_NATIVE channel layouts
if we want to add support to them.

...

I have no strong preference but I tend to prefer solution 1) as it
looks the safest to me, but I need to hear more opinions, especially
from devs which designed / are working with channel layout conversion
issues and may be aware of problems which I don't see.