diff mbox

[FFmpeg-devel,v2] avfilter/f_select: yuv will use Y plane only for scenecut detect

Message ID 20190812020619.15180-1-lance.lmwang@gmail.com
State Superseded
Headers show

Commit Message

Lance Wang Aug. 12, 2019, 2:06 a.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavfilter/f_select.c                     |  3 ++-
 tests/ref/fate/filter-metadata-scenedetect | 16 ++++++++--------
 2 files changed, 10 insertions(+), 9 deletions(-)

Comments

Marton Balint Aug. 12, 2019, 9:03 p.m. UTC | #1
On Mon, 12 Aug 2019, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavfilter/f_select.c                     |  3 ++-
> tests/ref/fate/filter-metadata-scenedetect | 16 ++++++++--------
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/libavfilter/f_select.c b/libavfilter/f_select.c
> index 5c7372c976..25a36ff0fa 100644
> --- a/libavfilter/f_select.c
> +++ b/libavfilter/f_select.c
> @@ -211,7 +211,8 @@ static int config_input(AVFilterLink *inlink)
>     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);
> +    select->nb_planes = desc->flags & AV_PIX_FMT_FLAG_PLANAR ? 1 : av_pix_fmt_count_planes(inlink->format);
> +

This is very misleading even if it happens to work for the currently 
supported formats.

An is_yuv variable is good to have, because it says what we want, which is 
limit the number of planes only for YUV. So please use the previosuly 
suggested

     int is_yuv = !(desc->flags & AV_PIX_FMT_FLAG_RGB) &&
                  (desc->flags & AV_PIX_FMT_FLAG_PLANAR) &&
                  desc->nb_components >= 3;

(copied from tiff.c by the way) definition.

Thanks,
Marton


>     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;
> diff --git a/tests/ref/fate/filter-metadata-scenedetect b/tests/ref/fate/filter-metadata-scenedetect
> index 7ce2d6794e..36c033bc0e 100644
> --- a/tests/ref/fate/filter-metadata-scenedetect
> +++ b/tests/ref/fate/filter-metadata-scenedetect
> @@ -1,11 +1,11 @@
> pkt_pts=1620|tag:lavfi.scene_score=1.000000
> -pkt_pts=4140|tag:lavfi.scene_score=0.668643
> -pkt_pts=5800|tag:lavfi.scene_score=0.996721
> -pkt_pts=6720|tag:lavfi.scene_score=0.357390
> -pkt_pts=8160|tag:lavfi.scene_score=0.886268
> -pkt_pts=9760|tag:lavfi.scene_score=0.926219
> -pkt_pts=14080|tag:lavfi.scene_score=0.650033
> +pkt_pts=4140|tag:lavfi.scene_score=0.923403
> +pkt_pts=5800|tag:lavfi.scene_score=1.000000
> +pkt_pts=6720|tag:lavfi.scene_score=0.475643
> +pkt_pts=8160|tag:lavfi.scene_score=1.000000
> +pkt_pts=9760|tag:lavfi.scene_score=1.000000
> +pkt_pts=14080|tag:lavfi.scene_score=0.874623
> pkt_pts=15700|tag:lavfi.scene_score=1.000000
> -pkt_pts=18500|tag:lavfi.scene_score=0.316402
> -pkt_pts=20040|tag:lavfi.scene_score=0.269509
> +pkt_pts=18500|tag:lavfi.scene_score=0.422509
> +pkt_pts=20040|tag:lavfi.scene_score=0.352360
> pkt_pts=21760|tag:lavfi.scene_score=1.000000
> -- 
> 2.21.0
>
> _______________________________________________
> 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".
Lance Wang Aug. 13, 2019, 1:51 a.m. UTC | #2
On Mon, Aug 12, 2019 at 11:03:24PM +0200, Marton Balint wrote:
> 
> 
> On Mon, 12 Aug 2019, lance.lmwang@gmail.com wrote:
> 
> >From: Limin Wang <lance.lmwang@gmail.com>
> >
> >Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >---
> >libavfilter/f_select.c                     |  3 ++-
> >tests/ref/fate/filter-metadata-scenedetect | 16 ++++++++--------
> >2 files changed, 10 insertions(+), 9 deletions(-)
> >
> >diff --git a/libavfilter/f_select.c b/libavfilter/f_select.c
> >index 5c7372c976..25a36ff0fa 100644
> >--- a/libavfilter/f_select.c
> >+++ b/libavfilter/f_select.c
> >@@ -211,7 +211,8 @@ static int config_input(AVFilterLink *inlink)
> >    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);
> >+    select->nb_planes = desc->flags & AV_PIX_FMT_FLAG_PLANAR ? 1 : av_pix_fmt_count_planes(inlink->format);
> >+
> 
> This is very misleading even if it happens to work for the currently
> supported formats.
> 
> An is_yuv variable is good to have, because it says what we want,
> which is limit the number of planes only for YUV. So please use the
> previosuly suggested
> 
>     int is_yuv = !(desc->flags & AV_PIX_FMT_FLAG_RGB) &&
>                  (desc->flags & AV_PIX_FMT_FLAG_PLANAR) &&
>                  desc->nb_components >= 3;
> 

OK, it's more clean, please checking the last update version.(please ignore the first two)


> (copied from tiff.c by the way) definition.
> 
> Thanks,
> Marton
> 
> 
> >    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;
> >diff --git a/tests/ref/fate/filter-metadata-scenedetect b/tests/ref/fate/filter-metadata-scenedetect
> >index 7ce2d6794e..36c033bc0e 100644
> >--- a/tests/ref/fate/filter-metadata-scenedetect
> >+++ b/tests/ref/fate/filter-metadata-scenedetect
> >@@ -1,11 +1,11 @@
> >pkt_pts=1620|tag:lavfi.scene_score=1.000000
> >-pkt_pts=4140|tag:lavfi.scene_score=0.668643
> >-pkt_pts=5800|tag:lavfi.scene_score=0.996721
> >-pkt_pts=6720|tag:lavfi.scene_score=0.357390
> >-pkt_pts=8160|tag:lavfi.scene_score=0.886268
> >-pkt_pts=9760|tag:lavfi.scene_score=0.926219
> >-pkt_pts=14080|tag:lavfi.scene_score=0.650033
> >+pkt_pts=4140|tag:lavfi.scene_score=0.923403
> >+pkt_pts=5800|tag:lavfi.scene_score=1.000000
> >+pkt_pts=6720|tag:lavfi.scene_score=0.475643
> >+pkt_pts=8160|tag:lavfi.scene_score=1.000000
> >+pkt_pts=9760|tag:lavfi.scene_score=1.000000
> >+pkt_pts=14080|tag:lavfi.scene_score=0.874623
> >pkt_pts=15700|tag:lavfi.scene_score=1.000000
> >-pkt_pts=18500|tag:lavfi.scene_score=0.316402
> >-pkt_pts=20040|tag:lavfi.scene_score=0.269509
> >+pkt_pts=18500|tag:lavfi.scene_score=0.422509
> >+pkt_pts=20040|tag:lavfi.scene_score=0.352360
> >pkt_pts=21760|tag:lavfi.scene_score=1.000000
> >-- 
> >2.21.0
> >
> >_______________________________________________
> >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".
> _______________________________________________
> 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".
diff mbox

Patch

diff --git a/libavfilter/f_select.c b/libavfilter/f_select.c
index 5c7372c976..25a36ff0fa 100644
--- a/libavfilter/f_select.c
+++ b/libavfilter/f_select.c
@@ -211,7 +211,8 @@  static int config_input(AVFilterLink *inlink)
     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);
+    select->nb_planes = desc->flags & AV_PIX_FMT_FLAG_PLANAR ? 1 : 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;
diff --git a/tests/ref/fate/filter-metadata-scenedetect b/tests/ref/fate/filter-metadata-scenedetect
index 7ce2d6794e..36c033bc0e 100644
--- a/tests/ref/fate/filter-metadata-scenedetect
+++ b/tests/ref/fate/filter-metadata-scenedetect
@@ -1,11 +1,11 @@ 
 pkt_pts=1620|tag:lavfi.scene_score=1.000000
-pkt_pts=4140|tag:lavfi.scene_score=0.668643
-pkt_pts=5800|tag:lavfi.scene_score=0.996721
-pkt_pts=6720|tag:lavfi.scene_score=0.357390
-pkt_pts=8160|tag:lavfi.scene_score=0.886268
-pkt_pts=9760|tag:lavfi.scene_score=0.926219
-pkt_pts=14080|tag:lavfi.scene_score=0.650033
+pkt_pts=4140|tag:lavfi.scene_score=0.923403
+pkt_pts=5800|tag:lavfi.scene_score=1.000000
+pkt_pts=6720|tag:lavfi.scene_score=0.475643
+pkt_pts=8160|tag:lavfi.scene_score=1.000000
+pkt_pts=9760|tag:lavfi.scene_score=1.000000
+pkt_pts=14080|tag:lavfi.scene_score=0.874623
 pkt_pts=15700|tag:lavfi.scene_score=1.000000
-pkt_pts=18500|tag:lavfi.scene_score=0.316402
-pkt_pts=20040|tag:lavfi.scene_score=0.269509
+pkt_pts=18500|tag:lavfi.scene_score=0.422509
+pkt_pts=20040|tag:lavfi.scene_score=0.352360
 pkt_pts=21760|tag:lavfi.scene_score=1.000000