[FFmpeg-devel] [PATCH v3] area changed: scdet filter

[radu.taraibuta at gmail.com]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Michael Niedermayer
> Sent: joi, 13 iunie 2024 02:52
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v3] area changed: scdet filter
> 
> 
> I honestly dont see the value of refcounting the global cbrt tables
> 
> either they can be static and initialized by a single AV_ONCE_INIT or just
> allocate and reinint for each filter.
> 

There is one cbrt_table for each bitdepth. Initializing all of them before knowing which one is used, would be a waste.
To allocate and initialize one table for each filter/client would be a waste too in a multiple clients scenario.

> > +
> > +void ff_scene_scrd_c(SCENE_SAD_PARAMS) {
> > +    uint64_t scrdPlus = 0;
> > +    uint64_t scrdMinus = 0;
> > +    int x, y;
> > +    double mean;
> > +    uint8_t *table = cbrt_table[8];
> > +
> > +    if (!table) {
> > +        *sum = 0;
> > +        return;
> > +    }
> > +
> > +    for (y = 0; y < height; y++) {
> > +        for (x = 0; x < width; x++)
> > +            if (src1[x] > src2[x])
> > +                scrdMinus += table[src1[x] - src2[x]];
> > +            else
> > +                scrdPlus += table[src2[x] - src1[x]];
> > +        src1 += stride1;
> > +        src2 += stride2;
> > +    }
> > +
> 
> > +    mean = (sqrt(scrdPlus) + sqrt(scrdMinus)) / 2.0;
> > +    *sum = 2.0 * mean * mean;
> 
> one sqrt less:
> 0.5*(scrdPlus + scrdMinus) + sqrt(scrdPlus * (double)scrdMinus)
> 

OK, will do in the next version.

> > +}
> > +
> > +void ff_scene_scrd2B_c(SCENE_SAD_PARAMS, int bitdepth) {
> > +    uint64_t scrdPlus = 0;
> > +    uint64_t scrdMinus = 0;
> > +    const uint16_t *src1w = (const uint16_t*)src1;
> > +    const uint16_t *src2w = (const uint16_t*)src2;
> > +    int x, y;
> > +    double mean;
> > +    uint16_t *table = (uint16_t*)cbrt_table[bitdepth];
> > +
> > +    if (!table) {
> > +        *sum = 0;
> > +        return;
> > +    }
> > +
> > +    stride1 /= 2;
> > +    stride2 /= 2;
> > +
> > +    for (y = 0; y < height; y++) {
> > +        for (x = 0; x < width; x++)
> > +            if (src1w[x] > src2w[x])
> > +                scrdMinus += table[src1w[x] - src2w[x]];
> > +            else
> > +                scrdPlus += table[src2w[x] - src1w[x]];
> > +        src1w += stride1;
> > +        src2w += stride2;
> > +    }
> > +
> > +    mean = (sqrt(scrdPlus) + sqrt(scrdMinus)) / 2.0;
> > +    *sum = 2.0 * mean * mean;
> > +}
> > +
> > +void ff_scene_scrd9_c(SCENE_SAD_PARAMS)
> > +{
> > +    ff_scene_scrd2B_c(src1, stride1, src2, stride2, width, height,
> > +sum, 9); }
> > +
> > +void ff_scene_scrd10_c(SCENE_SAD_PARAMS)
> > +{
> > +    ff_scene_scrd2B_c(src1, stride1, src2, stride2, width, height,
> > +sum, 10); }
> > +
> > +void ff_scene_scrd12_c(SCENE_SAD_PARAMS)
> > +{
> > +    ff_scene_scrd2B_c(src1, stride1, src2, stride2, width, height,
> > +sum, 12); }
> > +
> > +void ff_scene_scrd14_c(SCENE_SAD_PARAMS)
> > +{
> > +    ff_scene_scrd2B_c(src1, stride1, src2, stride2, width, height,
> > +sum, 14); }
> > +
> > +void ff_scene_scrd16_c(SCENE_SAD_PARAMS)
> > +{
> > +    ff_scene_scrd2B_c(src1, stride1, src2, stride2, width, height,
> > +sum, 16); }
> 
> duplicating this 5 times just for a different LUT pointer is ugly
> 
> 

It's not pretty, but I did it to preserve the existing interface and keep logic together.
Any suggestion how else to do it?