[FFmpeg-devel] PixFmtInfo cleanup

[Daniel Verkamp]

On Sun, Feb 22, 2009 at 4:30 PM, Stefano Sabatini
<stefano.sabatini-lala at poste.it> wrote:
> On date Thursday 2009-02-19 18:52:23 +0100, Michael Niedermayer encoded:
>> On Thu, Feb 19, 2009 at 06:10:50PM +0100, Vitor Sessak wrote:
>> > Michael Niedermayer wrote:
>> > > On Thu, Feb 19, 2009 at 01:19:22AM +0100, Stefano Sabatini wrote:
>> > >> On date Thursday 2009-02-19 00:20:23 +0100, Michael Niedermayer encoded:
>> > >>> On Thu, Feb 19, 2009 at 12:03:26AM +0100, Stefano Sabatini wrote:
>> > >>>> On date Sunday 2009-02-15 20:58:35 +0100, Vitor Sessak encoded:
>> > >>>>> Stefano Sabatini wrote:
> [...]
>> > >>>> My idea was to move the macros to lavu, redefine them something like:
>> > >>>> AV_PIX_FMT_IS_PLANAR_YUV()
>> > >>>> AV_PIX_FMT_IS_YUV()
>> > >>>> AV_PIX_FMT_IS_GRAY()
>> > >>>> ...
>> > >>>>
>> > >>>> and redefine the current libsws macros like:
>> > >>>> #define isPlanarYUV(x) AV_PIX_FMT_IS_PLANAR_YUV(x)
>> > >>>>
>> > >>>> swscale_internal.h already depends on libavutil/avutil.h.
>> > >>>>
>> > >>>> Would be this a acceptable?
>> > >>> first the macros in sws are inefficiently implemented,
>> > >>> an array so that
>> > >>>
>> > >>> AV_PIX_FMT_IS_YUV(x) (array[x]&0x01)
>> >
>> > Why using macros at all and not inline functions?
>>
>> because the call overhead exceeds the amount of code and i dont trust
>> the compiler
>> >
>> > >>> would be a good idea, also see pix_fmt_info
>> > >> Yes, I'm thinking about to move PixFmtInfo to lavu too.
>> >
>> > I like this idea. There are a lot of filters that could use data from
>> > PixFmtInfo.
>> >
>> > >>> if its well and clean implemenetd lavu is an option also sws should n that
>> > >>> case use lavus variant directly and not use wraper macros
>> > >> First step creates a pix_fmt.h header (PixFmtInfo would then be added
>> > >> to libavutil/pix_fmt.c)
>> > >
>> > > PixFmtInfo as is is too bloated, it requires cleanup _first_
>> >
>> > What is bloated/ugly about it? The only thing I see that could be
>> > improved is putting all the FF_COLOR_XXX and FF_PIXEL_XXX in an enum each...
>>
>> 12 bytes per pix_fmt for roughly 2 byte of actual information
>
> Well, I can think of this kind of compression:
>
> #define FF_COLOR_RGB          0 ///< RGB color space
> #define FF_COLOR_GRAY         1 ///< gray color space
> #define FF_COLOR_YUV          2 ///< YUV color space. 16 <= Y <= 235, 16 <= U, V <= 240
> #define FF_COLOR_YUV_JPEG     3 ///< YUV color space. 0 <= Y <= 255, 0 <= U, V <= 255
>
> #define FF_PIXEL_PLANAR       4 ///< each channel has one component in AVPicture
> #define FF_PIXEL_PACKED       5 ///< only one components containing all the channels
> #define FF_PIXEL_PALETTE      6 ///< one components containing indexes for a palette
>
> #define FF_PIXEL_HAS_ALPHA    7 ///< true if alpha can be specified

Maybe you mean

#define FF_COLOR_RGB          1 ///< RGB color space
#define FF_COLOR_GRAY         2 ///< gray color space
#define FF_COLOR_YUV          4 ///< YUV color space. 16 <= Y <= 235,
16 <= U, V <= 240

etc.?  As is, they won't work as flags, ex. FF_PIXEL_HAS_ALPHA (7) ==
1 | 2 | 4...

[...]

Thanks,
-- Daniel Verkamp