diff mbox

[FFmpeg-devel,v3,1/3] lavf/f_select: support scenecut with more pixel formats

Message ID 20190720232420.11994-1-lance.lmwang@gmail.com
State New
Headers show

Commit Message

Lance Wang July 20, 2019, 11:24 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

This patch haven't make other pixel format usable yet to make sure the test
result is same with rgb32 format.

Reviewed-by: Marton Balint <cus@passwd.hu>
Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavfilter/f_select.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

Comments

Marton Balint July 22, 2019, 9:13 p.m. UTC | #1
On Sun, 21 Jul 2019, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> This patch haven't make other pixel format usable yet to make sure the test
> result is same with rgb32 format.
>
> Reviewed-by: Marton Balint <cus@passwd.hu>

You should only add this if you got an explicit LGTM for a patch.

> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavfilter/f_select.c | 34 ++++++++++++++++++++++++++++++----
> 1 file changed, 30 insertions(+), 4 deletions(-)

Ok, I applied the series with some minor fixes and rewordings of your 
commit messages, as they were a bit difficult to understand sometimes.

Thanks,
Marton
Lance Wang July 23, 2019, 1:40 a.m. UTC | #2
On Mon, Jul 22, 2019 at 11:13:46PM +0200, Marton Balint wrote:
> 
> On Sun, 21 Jul 2019, lance.lmwang@gmail.com wrote:
> 
> >From: Limin Wang <lance.lmwang@gmail.com>
> >
> >This patch haven't make other pixel format usable yet to make sure the test
> >result is same with rgb32 format.
> >
> >Reviewed-by: Marton Balint <cus@passwd.hu>
> 
> You should only add this if you got an explicit LGTM for a patch.

OK, got it. thanks for the review, it's very helpful.

> 
> >Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >---
> >libavfilter/f_select.c | 34 ++++++++++++++++++++++++++++++----
> >1 file changed, 30 insertions(+), 4 deletions(-)
> 
> Ok, I applied the series with some minor fixes and rewordings of
> your commit messages, as they were a bit difficult to understand
> sometimes.
OK, after the change, it looks better.

> 
> Thanks,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Gyan Doshi July 23, 2019, 4:28 a.m. UTC | #3
On 23-07-2019 02:43 AM, Marton Balint wrote:
>
> On Sun, 21 Jul 2019, lance.lmwang@gmail.com wrote:
>
>> From: Limin Wang <lance.lmwang@gmail.com>
>>
>> This patch haven't make other pixel format usable yet to make sure 
>> the test
>> result is same with rgb32 format.
>>
>> Reviewed-by: Marton Balint <cus@passwd.hu>
>
> You should only add this if you got an explicit LGTM for a patch.
>
>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> ---
>> libavfilter/f_select.c | 34 ++++++++++++++++++++++++++++++----
>> 1 file changed, 30 insertions(+), 4 deletions(-)
>
> Ok, I applied the series with some minor fixes and rewordings of your 
> commit messages, as they were a bit difficult to understand sometimes.

"avformat/f_select: support scenecut with more pixel formats"

This should have been avfilter/f_select.

Gyan
Marton Balint July 23, 2019, 7:50 a.m. UTC | #4
On Tue, 23 Jul 2019, Gyan wrote:

>
>
> On 23-07-2019 02:43 AM, Marton Balint wrote:
>>
>> On Sun, 21 Jul 2019, lance.lmwang@gmail.com wrote:
>>
>>> From: Limin Wang <lance.lmwang@gmail.com>
>>>
>>> This patch haven't make other pixel format usable yet to make sure 
>>> the test
>>> result is same with rgb32 format.
>>>
>>> Reviewed-by: Marton Balint <cus@passwd.hu>
>>
>> You should only add this if you got an explicit LGTM for a patch.
>>
>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>>> ---
>>> libavfilter/f_select.c | 34 ++++++++++++++++++++++++++++++----
>>> 1 file changed, 30 insertions(+), 4 deletions(-)
>>
>> Ok, I applied the series with some minor fixes and rewordings of your 
>> commit messages, as they were a bit difficult to understand sometimes.
>
> "avformat/f_select: support scenecut with more pixel formats"
>
> This should have been avfilter/f_select.

Yes, sorry about that.

There is one more thing that we might consider. Now the scene change 
detection score uses all planes to detect scene changes. In this regard 
this is similar how the frozen frames detection works. However, in 
classic encoding scene change detection typically only uses the Y plane as 
far as I know, not the chroma planes.

We might get more resonable scores for scene change if we also use only 
the Y plane for calculating the score if the pixel format is YUV. Although 
this will require additional work once packed YUV formats are added, 
because for the moment the generic scene sad score calculation has no way 
to ignore some components in a packed format. Also it does not make a 
whole lot of difference, because chroma is usually subsampled anyway, so 
it is weighting less in the scene score. So I guess there are pros and 
cons for this.

Regards,
Marton
diff mbox

Patch

diff --git a/libavfilter/f_select.c b/libavfilter/f_select.c
index 1132375..b872cee 100644
--- a/libavfilter/f_select.c
+++ b/libavfilter/f_select.c
@@ -28,6 +28,8 @@ 
 #include "libavutil/fifo.h"
 #include "libavutil/internal.h"
 #include "libavutil/opt.h"
+#include "libavutil/imgutils.h"
+#include "libavutil/pixdesc.h"
 #include "avfilter.h"
 #include "audio.h"
 #include "formats.h"
@@ -144,6 +146,10 @@  typedef struct SelectContext {
     char *expr_str;
     AVExpr *expr;
     double var_values[VAR_VARS_NB];
+    int bitdepth;
+    int nb_planes;
+    ptrdiff_t width[4];
+    ptrdiff_t height[4];
     int do_scene_detect;            ///< 1 if the expression requires scene detection variables, 0 otherwise
     ff_scene_sad_fn sad;            ///< Sum of the absolute difference function (scene detect only)
     double prev_mafd;               ///< previous MAFD                           (scene detect only)
@@ -202,6 +208,17 @@  static av_cold int init(AVFilterContext *ctx)
 static int config_input(AVFilterLink *inlink)
 {
     SelectContext *select = inlink->dst->priv;
+    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
+
+    select->bitdepth = desc->comp[0].depth;
+    select->nb_planes = av_pix_fmt_count_planes(inlink->format);
+    for (int plane = 0; plane < select->nb_planes; plane++) {
+        ptrdiff_t line_size = av_image_get_linesize(inlink->format, inlink->w, plane);
+        int vsub = desc->log2_chroma_h;
+
+        select->width[plane] = line_size >> (select->bitdepth > 8);
+        select->height[plane] = plane == 1 || plane == 2 ?  AV_CEIL_RSHIFT(inlink->h, vsub) : inlink->h;
+    }
 
     select->var_values[VAR_N]          = 0.0;
     select->var_values[VAR_SELECTED_N] = 0.0;
@@ -242,7 +259,7 @@  static int config_input(AVFilterLink *inlink)
         inlink->type == AVMEDIA_TYPE_AUDIO ? inlink->sample_rate : NAN;
 
     if (CONFIG_SELECT_FILTER && select->do_scene_detect) {
-        select->sad = ff_scene_sad_get_fn(8);
+        select->sad = ff_scene_sad_get_fn(select->bitdepth == 8 ? 8 : 16);
         if (!select->sad)
             return AVERROR(EINVAL);
     }
@@ -258,12 +275,21 @@  static double get_scene_score(AVFilterContext *ctx, AVFrame *frame)
     if (prev_picref &&
         frame->height == prev_picref->height &&
         frame->width  == prev_picref->width) {
-        uint64_t sad;
+        uint64_t sad = 0;
         double mafd, diff;
+        int count = 0;
+
+        for (int plane = 0; plane < select->nb_planes; plane++) {
+            uint64_t plane_sad;
+            select->sad(prev_picref->data[plane], prev_picref->linesize[plane],
+                    frame->data[plane], frame->linesize[plane],
+                    select->width[plane], select->height[plane], &plane_sad);
+            sad += plane_sad;
+            count += select->width[plane] * select->height[plane];
+        }
 
-        select->sad(prev_picref->data[0], prev_picref->linesize[0], frame->data[0], frame->linesize[0], frame->width * 3, frame->height, &sad);
         emms_c();
-        mafd = (double)sad / (frame->width * 3 * frame->height);
+        mafd = (double)sad / count / (1ULL << (select->bitdepth - 8));
         diff = fabs(mafd - select->prev_mafd);
         ret  = av_clipf(FFMIN(mafd, diff) / 100., 0, 1);
         select->prev_mafd = mafd;