Hi,

On Sat, Jan 7, 2012 at 1:00 PM, Diego Biurrun <[email protected]> wrote:
> On Sat, Jan 07, 2012 at 12:40:47PM -0800, Ronald S. Bultje wrote:
>> On Wed, Jan 4, 2012 at 6:14 AM, Diego Biurrun <[email protected]> wrote:
>> > --- a/libswscale/colorspace-test.c
>> > +++ b/libswscale/colorspace-test.c
>> > @@ -47,7 +47,7 @@ int main(int argc, char **argv)
>> >     av_log(NULL, AV_LOG_INFO, "memory corruption test ...\n");
>> >     sws_rgb2rgb_init();
>> >
>> > -    for(funcNum=0; ; funcNum++) {
>> > +    for (funcNum = 0; /* NOTHING */; funcNum++) {
>>
>> int i; // loop counter
>>
>> Or in short, the /* NOTHING */ comment smells like "this isn't really
>> needed", and we don't do that elsewhere either. The space changes are
>> OK.
>
> Not sure what you want to tell me here.

Remove the "/* NOTHING */".

>> > --- a/libswscale/swscale-test.c
>> > +++ b/libswscale/swscale-test.c
>> > @@ -35,33 +35,32 @@
>> >
>> >  /* HACK Duplicated from swscale_internal.h.
>> >  * Should be removed when a cleaner pixel format system exists. */
>> > -#define isGray(x)       (           \
>> > -           (x)==PIX_FMT_GRAY8       \
>> > -        || (x)==PIX_FMT_GRAY16BE    \
>> > -        || (x)==PIX_FMT_GRAY16LE    \
>> > -    )
>> > -#define hasChroma(x)   (!(          \
>> > -            isGray(x)               \
>> > -        || (x)==PIX_FMT_MONOBLACK   \
>> > -        || (x)==PIX_FMT_MONOWHITE   \
>> > -    ))
>> > -#define isALPHA(x)      (           \
>> > -           (x)==PIX_FMT_BGR32       \
>> > -        || (x)==PIX_FMT_BGR32_1     \
>> > -        || (x)==PIX_FMT_RGB32       \
>> > -        || (x)==PIX_FMT_RGB32_1     \
>> > -        || (x)==PIX_FMT_YUVA420P    \
>> > -    )
>> > -
>> > -static uint64_t getSSD(uint8_t *src1, uint8_t *src2, int stride1, int 
>> > stride2, int w, int h)
>> > +#define isGray(x)                      \
>> > +    ((x) == PIX_FMT_GRAY8       ||     \
>> > +     (x) == PIX_FMT_Y400A       ||     \
>> > +     (x) == PIX_FMT_GRAY16BE    ||     \
>> > +     (x) == PIX_FMT_GRAY16LE)
>> > +#define hasChroma(x)                   \
>> > +    (!(isGray(x)                ||     \
>> > +       (x) == PIX_FMT_MONOBLACK ||     \
>> > +       (x) == PIX_FMT_MONOWHITE))
>> > +#define isALPHA(x)                     \
>> > +    ((x) == PIX_FMT_BGR32   ||         \
>> > +     (x) == PIX_FMT_BGR32_1 ||         \
>> > +     (x) == PIX_FMT_RGB32   ||         \
>> > +     (x) == PIX_FMT_RGB32_1 ||         \
>> > +     (x) == PIX_FMT_YUVA420P)
>>
>> Shall we fix the hack instead?
>
> Maybe after I have pushed my patch?  And how without swscale_internal.h
> dependencies?

Later is fine. The idea was to move them to libavutil after they're
properly namespaced and cleaned up. Most of these macros are clean
now, but a few hacks remain and a few of them are still just a list of
pixelformats.

(Alternatively, move them out of swscale_internal.h into a new header,
but that's way too much effort for code that will disappear anyway.)

Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to