diff mbox series

[FFmpeg-devel,06/12] fftools/ffmpeg_dec: apply cropping manually

Message ID 20240322202841.31730-6-anton@khirnov.net
State Accepted
Commit b1aaa1f585d3a69bb7a0972cbec9a8c71cce5610
Headers show
Series [FFmpeg-devel,01/12] tests/fate/ffmpeg: evaluate thread count in fate-run.sh rather than make | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov March 22, 2024, 8:28 p.m. UTC 
lavfi does not require aligned buffers, so we can safely apply top/left
cropping by any amount, without passing any special flags to lavc.
Longer term, an even better solution would probably be auto-inserting
the crop filter (or its hwaccel versions) as needed.

Multiple FATE tests no longer need -flags unaligned.
---
 fftools/ffmpeg_dec.c | 14 ++++++++++++++
 tests/fate/h264.mak  |  2 +-
 tests/fate/hevc.mak  |  8 ++++----
 tests/fate/vvc.mak   |  2 +-
 4 files changed, 20 insertions(+), 6 deletions(-)

Comments

James Almer March 23, 2024, 2:53 a.m. UTC | #1 
On 3/22/2024 5:28 PM, Anton Khirnov wrote:
> lavfi does not require aligned buffers, so we can safely apply top/left
> cropping by any amount, without passing any special flags to lavc.
> Longer term, an even better solution would probably be auto-inserting
> the crop filter (or its hwaccel versions) as needed.

It's what i did in my container cropping set. One problem with it is 
that the cropping filter doesn't look at cropping values in input frames 
but at filter options instead, and will in fact *add* cropping values to 
the output frames if you pass it something it can't handle, like hwaccel 
formats.

And for that matter, cropping fields in frames should be handled by 
encoders, or by the generic encoding code in some form. Right now they 
are outright ignored.

> 
> Multiple FATE tests no longer need -flags unaligned.
> ---
>   fftools/ffmpeg_dec.c | 14 ++++++++++++++
>   tests/fate/h264.mak  |  2 +-
>   tests/fate/hevc.mak  |  8 ++++----
>   tests/fate/vvc.mak   |  2 +-
>   4 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
> index b8bfae469f..e3ed0b3978 100644
> --- a/fftools/ffmpeg_dec.c
> +++ b/fftools/ffmpeg_dec.c
> @@ -51,6 +51,7 @@ typedef struct DecoderPriv {
>   
>       // a combination of DECODER_FLAG_*, provided to dec_open()
>       int                 flags;
> +    int                 apply_cropping;
>   
>       enum AVPixelFormat  hwaccel_pix_fmt;
>       enum HWAccelID      hwaccel_id;
> @@ -404,6 +405,15 @@ static int video_frame_process(DecoderPriv *dp, AVFrame *frame)
>       if (dp->sar_override.num)
>           frame->sample_aspect_ratio = dp->sar_override;
>   
> +    if (dp->apply_cropping) {
> +        // lavfi does not require aligned frame data
> +        int ret = av_frame_apply_cropping(frame, AV_FRAME_CROP_UNALIGNED);
> +        if (ret < 0) {
> +            av_log(dp, AV_LOG_ERROR, "Error applying decoder cropping\n");
> +            return ret;
> +        }
> +    }
> +
>       return 0;
>   }
>   
> @@ -1214,6 +1224,10 @@ static int dec_open(DecoderPriv *dp, AVDictionary **dec_opts,
>       if (o->flags & DECODER_FLAG_BITEXACT)
>           dp->dec_ctx->flags |= AV_CODEC_FLAG_BITEXACT;
>   
> +    // we apply cropping outselves
> +    dp->apply_cropping          = dp->dec_ctx->apply_cropping;
> +    dp->dec_ctx->apply_cropping = 0;
> +
>       if ((ret = avcodec_open2(dp->dec_ctx, codec, NULL)) < 0) {
>           av_log(dp, AV_LOG_ERROR, "Error while opening decoder: %s\n",
>                  av_err2str(ret));
> diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
> index 674054560b..88f06d7b15 100644
> --- a/tests/fate/h264.mak
> +++ b/tests/fate/h264.mak
> @@ -312,7 +312,7 @@ fate-h264-conformance-ci1_ft_b:                   CMD = framecrc -i $(TARGET_SAM
>   fate-h264-conformance-ci_mw_d:                    CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CI_MW_D.264
>   fate-h264-conformance-cvbs3_sony_c:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVBS3_Sony_C.jsv
>   fate-h264-conformance-cvcanlma2_sony_c:           CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVCANLMA2_Sony_C.jsv
> -fate-h264-conformance-cvfc1_sony_c:               CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/h264-conformance/CVFC1_Sony_C.jsv
> +fate-h264-conformance-cvfc1_sony_c:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFC1_Sony_C.jsv
>   fate-h264-conformance-cvfi1_sony_d:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFI1_Sony_D.jsv
>   fate-h264-conformance-cvfi1_sva_c:                CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFI1_SVA_C.264
>   fate-h264-conformance-cvfi2_sony_h:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFI2_Sony_H.jsv
> diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
> index 4889ee8237..720603c112 100644
> --- a/tests/fate/hevc.mak
> +++ b/tests/fate/hevc.mak
> @@ -200,8 +200,8 @@ $(HEVC_TESTS_444_8BIT): SCALE_OPTS := -pix_fmt yuv444p
>   $(HEVC_TESTS_10BIT): SCALE_OPTS := -pix_fmt yuv420p10le -vf scale
>   $(HEVC_TESTS_422_10BIT) $(HEVC_TESTS_422_10BIN): SCALE_OPTS := -pix_fmt yuv422p10le -vf scale
>   $(HEVC_TESTS_444_12BIT): SCALE_OPTS := -pix_fmt yuv444p12le -vf scale
> -fate-hevc-conformance-%: CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bit $(SCALE_OPTS)
> -$(HEVC_TESTS_422_10BIN): CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bin $(SCALE_OPTS)
> +fate-hevc-conformance-%: CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bit $(SCALE_OPTS)
> +$(HEVC_TESTS_422_10BIN): CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bin $(SCALE_OPTS)
>   
>   FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER) += $(HEVC_TESTS_8BIT) $(HEVC_TESTS_444_8BIT)
>   FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER SCALE_FILTER) +=         \
> @@ -252,10 +252,10 @@ FATE_HEVC_FFPROBE-$(call DEMDEC, MOV, HEVC) += fate-hevc-dv-rpu
>   fate-hevc-two-first-slice: CMD = threads=2 framemd5 -i $(TARGET_SAMPLES)/hevc/two_first_slice.mp4 -sws_flags bitexact -t 00:02.00 -an
>   FATE_HEVC-$(call FRAMEMD5, MOV, HEVC) += fate-hevc-two-first-slice
>   
> -fate-hevc-cabac-tudepth: CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc/cbf_cr_cb_TUDepth_4_circle.h265 -pix_fmt yuv444p
> +fate-hevc-cabac-tudepth: CMD = framecrc -i $(TARGET_SAMPLES)/hevc/cbf_cr_cb_TUDepth_4_circle.h265 -pix_fmt yuv444p
>   FATE_HEVC-$(call FRAMECRC, HEVC, HEVC) += fate-hevc-cabac-tudepth
>   
> -fate-hevc-small422chroma: CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc/food.hevc -pix_fmt yuv422p10le -vf scale
> +fate-hevc-small422chroma: CMD = framecrc -i $(TARGET_SAMPLES)/hevc/food.hevc -pix_fmt yuv422p10le -vf scale
>   FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER SCALE_FILTER) += fate-hevc-small422chroma
>   
>   FATE_SAMPLES_AVCONV += $(FATE_HEVC-yes)
> diff --git a/tests/fate/vvc.mak b/tests/fate/vvc.mak
> index f5a45cc4ca..6cb3471b55 100644
> --- a/tests/fate/vvc.mak
> +++ b/tests/fate/vvc.mak
> @@ -38,7 +38,7 @@ $(foreach VAR,$(FATE_VVC_VARS), $(eval VVC_TESTS_$(VAR) := $(addprefix fate-vvc-
>   $(VVC_TESTS_8BIT): SCALE_OPTS := -pix_fmt yuv420p
>   $(VVC_TESTS_10BIT): SCALE_OPTS := -pix_fmt yuv420p10le -vf scale
>   $(VVC_TESTS_444_10BIT): SCALE_OPTS := -pix_fmt yuv444p10le -vf scale
> -fate-vvc-conformance-%: CMD = framecrc -flags unaligned -c:v vvc -strict experimental -i $(TARGET_SAMPLES)/vvc-conformance/$(subst fate-vvc-conformance-,,$(@)).bit $(SCALE_OPTS)
> +fate-vvc-conformance-%: CMD = framecrc -c:v vvc -strict experimental -i $(TARGET_SAMPLES)/vvc-conformance/$(subst fate-vvc-conformance-,,$(@)).bit $(SCALE_OPTS)
>   
>   FATE_VVC-$(call FRAMECRC, VVC, VVC, VVC_PARSER) += $(VVC_TESTS_8BIT)
>   FATE_VVC-$(call FRAMECRC, VVC, VVC, VVC_PARSER SCALE_FILTER) +=            \
Anton Khirnov March 23, 2024, 7:20 a.m. UTC | #2 
Quoting James Almer (2024-03-23 03:53:26)
> On 3/22/2024 5:28 PM, Anton Khirnov wrote:
> > lavfi does not require aligned buffers, so we can safely apply top/left
> > cropping by any amount, without passing any special flags to lavc.
> > Longer term, an even better solution would probably be auto-inserting
> > the crop filter (or its hwaccel versions) as needed.
> 
> It's what i did in my container cropping set. One problem with it is 
> that the cropping filter doesn't look at cropping values in input frames 
> but at filter options instead, and will in fact *add* cropping values to 
> the output frames if you pass it something it can't handle, like hwaccel 
> formats.

The idea is that ffmpeg_filter would set up the crop filter, then remove
the values from the frame. The crop filter then either actually performs
cropping or adds them back.

> And for that matter, cropping fields in frames should be handled by 
> encoders, or by the generic encoding code in some form. Right now they 
> are outright ignored.

Sure I guess. Patches welcome?
diff mbox series

Patch

diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
index b8bfae469f..e3ed0b3978 100644
--- a/fftools/ffmpeg_dec.c
+++ b/fftools/ffmpeg_dec.c
@@ -51,6 +51,7 @@  typedef struct DecoderPriv {
 
     // a combination of DECODER_FLAG_*, provided to dec_open()
     int                 flags;
+    int                 apply_cropping;
 
     enum AVPixelFormat  hwaccel_pix_fmt;
     enum HWAccelID      hwaccel_id;
@@ -404,6 +405,15 @@  static int video_frame_process(DecoderPriv *dp, AVFrame *frame)
     if (dp->sar_override.num)
         frame->sample_aspect_ratio = dp->sar_override;
 
+    if (dp->apply_cropping) {
+        // lavfi does not require aligned frame data
+        int ret = av_frame_apply_cropping(frame, AV_FRAME_CROP_UNALIGNED);
+        if (ret < 0) {
+            av_log(dp, AV_LOG_ERROR, "Error applying decoder cropping\n");
+            return ret;
+        }
+    }
+
     return 0;
 }
 
@@ -1214,6 +1224,10 @@  static int dec_open(DecoderPriv *dp, AVDictionary **dec_opts,
     if (o->flags & DECODER_FLAG_BITEXACT)
         dp->dec_ctx->flags |= AV_CODEC_FLAG_BITEXACT;
 
+    // we apply cropping outselves
+    dp->apply_cropping          = dp->dec_ctx->apply_cropping;
+    dp->dec_ctx->apply_cropping = 0;
+
     if ((ret = avcodec_open2(dp->dec_ctx, codec, NULL)) < 0) {
         av_log(dp, AV_LOG_ERROR, "Error while opening decoder: %s\n",
                av_err2str(ret));
diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
index 674054560b..88f06d7b15 100644
--- a/tests/fate/h264.mak
+++ b/tests/fate/h264.mak
@@ -312,7 +312,7 @@  fate-h264-conformance-ci1_ft_b:                   CMD = framecrc -i $(TARGET_SAM
 fate-h264-conformance-ci_mw_d:                    CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CI_MW_D.264
 fate-h264-conformance-cvbs3_sony_c:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVBS3_Sony_C.jsv
 fate-h264-conformance-cvcanlma2_sony_c:           CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVCANLMA2_Sony_C.jsv
-fate-h264-conformance-cvfc1_sony_c:               CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/h264-conformance/CVFC1_Sony_C.jsv
+fate-h264-conformance-cvfc1_sony_c:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFC1_Sony_C.jsv
 fate-h264-conformance-cvfi1_sony_d:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFI1_Sony_D.jsv
 fate-h264-conformance-cvfi1_sva_c:                CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFI1_SVA_C.264
 fate-h264-conformance-cvfi2_sony_h:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFI2_Sony_H.jsv
diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
index 4889ee8237..720603c112 100644
--- a/tests/fate/hevc.mak
+++ b/tests/fate/hevc.mak
@@ -200,8 +200,8 @@  $(HEVC_TESTS_444_8BIT): SCALE_OPTS := -pix_fmt yuv444p
 $(HEVC_TESTS_10BIT): SCALE_OPTS := -pix_fmt yuv420p10le -vf scale
 $(HEVC_TESTS_422_10BIT) $(HEVC_TESTS_422_10BIN): SCALE_OPTS := -pix_fmt yuv422p10le -vf scale
 $(HEVC_TESTS_444_12BIT): SCALE_OPTS := -pix_fmt yuv444p12le -vf scale
-fate-hevc-conformance-%: CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bit $(SCALE_OPTS)
-$(HEVC_TESTS_422_10BIN): CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bin $(SCALE_OPTS)
+fate-hevc-conformance-%: CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bit $(SCALE_OPTS)
+$(HEVC_TESTS_422_10BIN): CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bin $(SCALE_OPTS)
 
 FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER) += $(HEVC_TESTS_8BIT) $(HEVC_TESTS_444_8BIT)
 FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER SCALE_FILTER) +=         \
@@ -252,10 +252,10 @@  FATE_HEVC_FFPROBE-$(call DEMDEC, MOV, HEVC) += fate-hevc-dv-rpu
 fate-hevc-two-first-slice: CMD = threads=2 framemd5 -i $(TARGET_SAMPLES)/hevc/two_first_slice.mp4 -sws_flags bitexact -t 00:02.00 -an
 FATE_HEVC-$(call FRAMEMD5, MOV, HEVC) += fate-hevc-two-first-slice
 
-fate-hevc-cabac-tudepth: CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc/cbf_cr_cb_TUDepth_4_circle.h265 -pix_fmt yuv444p
+fate-hevc-cabac-tudepth: CMD = framecrc -i $(TARGET_SAMPLES)/hevc/cbf_cr_cb_TUDepth_4_circle.h265 -pix_fmt yuv444p
 FATE_HEVC-$(call FRAMECRC, HEVC, HEVC) += fate-hevc-cabac-tudepth
 
-fate-hevc-small422chroma: CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc/food.hevc -pix_fmt yuv422p10le -vf scale
+fate-hevc-small422chroma: CMD = framecrc -i $(TARGET_SAMPLES)/hevc/food.hevc -pix_fmt yuv422p10le -vf scale
 FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER SCALE_FILTER) += fate-hevc-small422chroma
 
 FATE_SAMPLES_AVCONV += $(FATE_HEVC-yes)
diff --git a/tests/fate/vvc.mak b/tests/fate/vvc.mak
index f5a45cc4ca..6cb3471b55 100644
--- a/tests/fate/vvc.mak
+++ b/tests/fate/vvc.mak
@@ -38,7 +38,7 @@  $(foreach VAR,$(FATE_VVC_VARS), $(eval VVC_TESTS_$(VAR) := $(addprefix fate-vvc-
 $(VVC_TESTS_8BIT): SCALE_OPTS := -pix_fmt yuv420p
 $(VVC_TESTS_10BIT): SCALE_OPTS := -pix_fmt yuv420p10le -vf scale
 $(VVC_TESTS_444_10BIT): SCALE_OPTS := -pix_fmt yuv444p10le -vf scale
-fate-vvc-conformance-%: CMD = framecrc -flags unaligned -c:v vvc -strict experimental -i $(TARGET_SAMPLES)/vvc-conformance/$(subst fate-vvc-conformance-,,$(@)).bit $(SCALE_OPTS)
+fate-vvc-conformance-%: CMD = framecrc -c:v vvc -strict experimental -i $(TARGET_SAMPLES)/vvc-conformance/$(subst fate-vvc-conformance-,,$(@)).bit $(SCALE_OPTS)
 
 FATE_VVC-$(call FRAMECRC, VVC, VVC, VVC_PARSER) += $(VVC_TESTS_8BIT)
 FATE_VVC-$(call FRAMECRC, VVC, VVC, VVC_PARSER SCALE_FILTER) +=            \