[FFmpeg-devel,v4,1/7] vf_crop: Add support for cropping hardware frames

Submitted by Mark Thompson on April 9, 2019, 10:07 p.m.

Details

Message ID 20190409220730.29311-1-sw@jkqxz.net
State New
Headers show

Commit Message

Mark Thompson April 9, 2019, 10:07 p.m.
Set the cropping fields in the AVFrame.
---
On 26/03/2019 10:59, Song, Ruiling wrote:> 
> I think we need to make scale_vaapi evaluate input dimensions considering crop information. What do you think?

I agree.  But the cropping information is currently carried on the frame, not at any higher level (from the codec context or on the filter link), so we don't have any idea how big the output frames need to be at setup time when we have to set the size on the output link.  Making it work requires carrying more complete information through filter setup, similar to the problem with colour range and other properties which aren't reflected in the existing format.  (This is on my to-do list.)


 libavfilter/vf_crop.c | 74 +++++++++++++++++++++++++++++--------------
 1 file changed, 51 insertions(+), 23 deletions(-)

Comments

Ruiling Song April 15, 2019, 3:35 a.m.
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of

> Mark Thompson

> Sent: Wednesday, April 10, 2019 6:07 AM

> To: ffmpeg-devel@ffmpeg.org

> Subject: [FFmpeg-devel] [PATCH v4 1/7] vf_crop: Add support for cropping

> hardware frames

> 

> Set the cropping fields in the AVFrame.

The patchset looks fine to me. But I am not quite sure if others are happy with this crop patch.
If nobody against. I think you can go pushing the patchset when you make sure it will not trigger build failures as reported by Michael.
(one unnecessary empty line below, please remove it)

Thanks!
Ruiling

> ---

> On 26/03/2019 10:59, Song, Ruiling wrote:>

> > I think we need to make scale_vaapi evaluate input dimensions considering

> crop information. What do you think?

> 

> I agree.  But the cropping information is currently carried on the frame, not at

> any higher level (from the codec context or on the filter link), so we don't have

> any idea how big the output frames need to be at setup time when we have to

> set the size on the output link.  Making it work requires carrying more complete

> information through filter setup, similar to the problem with colour range and

> other properties which aren't reflected in the existing format.  (This is on my to-

> do list.)

> 

> 

>  libavfilter/vf_crop.c | 74 +++++++++++++++++++++++++++++--------------

>  1 file changed, 51 insertions(+), 23 deletions(-)

> 

> diff --git a/libavfilter/vf_crop.c b/libavfilter/vf_crop.c

> index 84be4c7d0d..7f6b0f03d3 100644

> --- a/libavfilter/vf_crop.c

> +++ b/libavfilter/vf_crop.c

> @@ -98,9 +98,17 @@ static int query_formats(AVFilterContext *ctx)

> 

>      for (fmt = 0; av_pix_fmt_desc_get(fmt); fmt++) {

>          const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(fmt);

> -        if (!(desc->flags & (AV_PIX_FMT_FLAG_HWACCEL |

> AV_PIX_FMT_FLAG_BITSTREAM)) &&

> -            !((desc->log2_chroma_w || desc->log2_chroma_h) && !(desc->flags &

> AV_PIX_FMT_FLAG_PLANAR)) &&

> -            (ret = ff_add_format(&formats, fmt)) < 0)

> +        if (desc->flags & AV_PIX_FMT_FLAG_BITSTREAM)

> +            continue;

> +        if (!(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {

> +            // Not usable if there is any subsampling but the format is

> +            // not planar (e.g. YUYV422).

> +            if ((desc->log2_chroma_w || desc->log2_chroma_h) &&

> +                !(desc->flags & AV_PIX_FMT_FLAG_PLANAR))

> +                continue;

> +        }

> +        ret = ff_add_format(&formats, fmt);

> +        if (ret < 0)

>              return ret;

>      }

> 

> @@ -157,8 +165,14 @@ static int config_input(AVFilterLink *link)

>      s->var_values[VAR_POS]   = NAN;

> 

>      av_image_fill_max_pixsteps(s->max_step, NULL, pix_desc);

> -    s->hsub = pix_desc->log2_chroma_w;

> -    s->vsub = pix_desc->log2_chroma_h;

> +

> +    if (pix_desc->flags & AV_PIX_FMT_FLAG_HWACCEL) {

> +        s->hsub = 1;

> +        s->vsub = 1;

> +    } else {

> +        s->hsub = pix_desc->log2_chroma_w;

> +        s->vsub = pix_desc->log2_chroma_h;

> +    }

> 

>      if ((ret = av_expr_parse_and_eval(&res, (expr = s->w_expr),

>                                        var_names, s->var_values,

> @@ -237,9 +251,15 @@ fail_expr:

>  static int config_output(AVFilterLink *link)

>  {

>      CropContext *s = link->src->priv;

> +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);

> 

> -    link->w = s->w;

> -    link->h = s->h;

> +    if (desc->flags & AV_PIX_FMT_FLAG_HWACCEL) {

> +        // Hardware frames adjust the cropping regions rather than

> +        // changing the frame size.

> +    } else {

> +        link->w = s->w;

> +        link->h = s->h;

> +    }

>      link->sample_aspect_ratio = s->out_sar;

> 

>      return 0;

> @@ -252,9 +272,6 @@ static int filter_frame(AVFilterLink *link, AVFrame

> *frame)

>      const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);

>      int i;

> 

> -    frame->width  = s->w;

> -    frame->height = s->h;

> -

>      s->var_values[VAR_N] = link->frame_count_out;

>      s->var_values[VAR_T] = frame->pts == AV_NOPTS_VALUE ?

>          NAN : frame->pts * av_q2d(link->time_base);

> @@ -285,22 +302,33 @@ static int filter_frame(AVFilterLink *link, AVFrame

> *frame)

>              (int)s->var_values[VAR_N], s->var_values[VAR_T], s-

> >var_values[VAR_POS],

>              s->x, s->y, s->x+s->w, s->y+s->h);

> 

> -    frame->data[0] += s->y * frame->linesize[0];

> -    frame->data[0] += s->x * s->max_step[0];

> -

> -    if (!(desc->flags & AV_PIX_FMT_FLAG_PAL || desc->flags & FF_PSEUDOPAL))

> {

> -        for (i = 1; i < 3; i ++) {

> -            if (frame->data[i]) {

> -                frame->data[i] += (s->y >> s->vsub) * frame->linesize[i];

> -                frame->data[i] += (s->x * s->max_step[i]) >> s->hsub;

> +    if (desc->flags & AV_PIX_FMT_FLAG_HWACCEL) {

> +        frame->crop_top   += s->y;

> +        frame->crop_left  += s->x;

> +        frame->crop_bottom = frame->height - frame->crop_top - frame-

> >crop_bottom - s->h;

> +        frame->crop_right  = frame->width  - frame->crop_left - frame-

> >crop_right - s->w;

> +

Unnecessary empty line above.

> +    } else {

> +        frame->width  = s->w;

> +        frame->height = s->h;

> +

> +        frame->data[0] += s->y * frame->linesize[0];

> +        frame->data[0] += s->x * s->max_step[0];

> +

> +        if (!(desc->flags & AV_PIX_FMT_FLAG_PAL || desc->flags &

> FF_PSEUDOPAL)) {

> +            for (i = 1; i < 3; i ++) {

> +                if (frame->data[i]) {

> +                    frame->data[i] += (s->y >> s->vsub) * frame->linesize[i];

> +                    frame->data[i] += (s->x * s->max_step[i]) >> s->hsub;

> +                }

>              }

>          }

> -    }

> 

> -    /* alpha plane */

> -    if (frame->data[3]) {

> -        frame->data[3] += s->y * frame->linesize[3];

> -        frame->data[3] += s->x * s->max_step[3];

> +        /* alpha plane */

> +        if (frame->data[3]) {

> +            frame->data[3] += s->y * frame->linesize[3];

> +            frame->data[3] += s->x * s->max_step[3];

> +        }

>      }

> 

>      return ff_filter_frame(link->dst->outputs[0], frame);

> --

> 2.20.1

> 

> _______________________________________________

> 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".

Patch hide | download patch | download mbox

diff --git a/libavfilter/vf_crop.c b/libavfilter/vf_crop.c
index 84be4c7d0d..7f6b0f03d3 100644
--- a/libavfilter/vf_crop.c
+++ b/libavfilter/vf_crop.c
@@ -98,9 +98,17 @@  static int query_formats(AVFilterContext *ctx)
 
     for (fmt = 0; av_pix_fmt_desc_get(fmt); fmt++) {
         const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(fmt);
-        if (!(desc->flags & (AV_PIX_FMT_FLAG_HWACCEL | AV_PIX_FMT_FLAG_BITSTREAM)) &&
-            !((desc->log2_chroma_w || desc->log2_chroma_h) && !(desc->flags & AV_PIX_FMT_FLAG_PLANAR)) &&
-            (ret = ff_add_format(&formats, fmt)) < 0)
+        if (desc->flags & AV_PIX_FMT_FLAG_BITSTREAM)
+            continue;
+        if (!(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
+            // Not usable if there is any subsampling but the format is
+            // not planar (e.g. YUYV422).
+            if ((desc->log2_chroma_w || desc->log2_chroma_h) &&
+                !(desc->flags & AV_PIX_FMT_FLAG_PLANAR))
+                continue;
+        }
+        ret = ff_add_format(&formats, fmt);
+        if (ret < 0)
             return ret;
     }
 
@@ -157,8 +165,14 @@  static int config_input(AVFilterLink *link)
     s->var_values[VAR_POS]   = NAN;
 
     av_image_fill_max_pixsteps(s->max_step, NULL, pix_desc);
-    s->hsub = pix_desc->log2_chroma_w;
-    s->vsub = pix_desc->log2_chroma_h;
+
+    if (pix_desc->flags & AV_PIX_FMT_FLAG_HWACCEL) {
+        s->hsub = 1;
+        s->vsub = 1;
+    } else {
+        s->hsub = pix_desc->log2_chroma_w;
+        s->vsub = pix_desc->log2_chroma_h;
+    }
 
     if ((ret = av_expr_parse_and_eval(&res, (expr = s->w_expr),
                                       var_names, s->var_values,
@@ -237,9 +251,15 @@  fail_expr:
 static int config_output(AVFilterLink *link)
 {
     CropContext *s = link->src->priv;
+    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
 
-    link->w = s->w;
-    link->h = s->h;
+    if (desc->flags & AV_PIX_FMT_FLAG_HWACCEL) {
+        // Hardware frames adjust the cropping regions rather than
+        // changing the frame size.
+    } else {
+        link->w = s->w;
+        link->h = s->h;
+    }
     link->sample_aspect_ratio = s->out_sar;
 
     return 0;
@@ -252,9 +272,6 @@  static int filter_frame(AVFilterLink *link, AVFrame *frame)
     const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
     int i;
 
-    frame->width  = s->w;
-    frame->height = s->h;
-
     s->var_values[VAR_N] = link->frame_count_out;
     s->var_values[VAR_T] = frame->pts == AV_NOPTS_VALUE ?
         NAN : frame->pts * av_q2d(link->time_base);
@@ -285,22 +302,33 @@  static int filter_frame(AVFilterLink *link, AVFrame *frame)
             (int)s->var_values[VAR_N], s->var_values[VAR_T], s->var_values[VAR_POS],
             s->x, s->y, s->x+s->w, s->y+s->h);
 
-    frame->data[0] += s->y * frame->linesize[0];
-    frame->data[0] += s->x * s->max_step[0];
-
-    if (!(desc->flags & AV_PIX_FMT_FLAG_PAL || desc->flags & FF_PSEUDOPAL)) {
-        for (i = 1; i < 3; i ++) {
-            if (frame->data[i]) {
-                frame->data[i] += (s->y >> s->vsub) * frame->linesize[i];
-                frame->data[i] += (s->x * s->max_step[i]) >> s->hsub;
+    if (desc->flags & AV_PIX_FMT_FLAG_HWACCEL) {
+        frame->crop_top   += s->y;
+        frame->crop_left  += s->x;
+        frame->crop_bottom = frame->height - frame->crop_top - frame->crop_bottom - s->h;
+        frame->crop_right  = frame->width  - frame->crop_left - frame->crop_right - s->w;
+
+    } else {
+        frame->width  = s->w;
+        frame->height = s->h;
+
+        frame->data[0] += s->y * frame->linesize[0];
+        frame->data[0] += s->x * s->max_step[0];
+
+        if (!(desc->flags & AV_PIX_FMT_FLAG_PAL || desc->flags & FF_PSEUDOPAL)) {
+            for (i = 1; i < 3; i ++) {
+                if (frame->data[i]) {
+                    frame->data[i] += (s->y >> s->vsub) * frame->linesize[i];
+                    frame->data[i] += (s->x * s->max_step[i]) >> s->hsub;
+                }
             }
         }
-    }
 
-    /* alpha plane */
-    if (frame->data[3]) {
-        frame->data[3] += s->y * frame->linesize[3];
-        frame->data[3] += s->x * s->max_step[3];
+        /* alpha plane */
+        if (frame->data[3]) {
+            frame->data[3] += s->y * frame->linesize[3];
+            frame->data[3] += s->x * s->max_step[3];
+        }
     }
 
     return ff_filter_frame(link->dst->outputs[0], frame);