diff mbox series

[FFmpeg-devel,crop,support,for,matroska,demuxer,V4,2/3] libavcodec: Public code to support container crop

Message ID 20221104141336.1812-2-ovchinnikov.dmitrii@gmail.com
State New
Headers show
Series [FFmpeg-devel,crop,support,for,matroska,demuxer,V4,1/3] libavcodec: Add crop related fields to structure AVCodecContext and AVCodecParameters. | expand

Checks

Context Check Description
yinshiyou/make_fate_loongarch64 success Make fate finished
yinshiyou/make_loongarch64 warning New warnings during build

Commit Message

Dmitrii Ovchinnikov Nov. 4, 2022, 2:13 p.m. UTC 
Support both simple and receive_frame api
The container crop information is applied additional to frame crop information
---
 libavcodec/codec_par.c | 30 ++++++++++++++---------
 libavcodec/decode.c    | 54 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 11 deletions(-)

Comments

Michael Niedermayer Nov. 4, 2022, 3:34 p.m. UTC | #1 
On Fri, Nov 04, 2022 at 03:13:35PM +0100, OvchinnikovDmitrii wrote:
> Support both simple and receive_frame api
> The container crop information is applied additional to frame crop information
[...]
> +            av_log(avctx, AV_LOG_WARNING,
> +                "Apply container  and elementary stream corpping parametes error: "
> +                "container cropping parameters"
> +                "%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER" "
> +                "elementary stream croping paramters"
> +                "%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER" "
> +                "(frame size %dx%d). This is a bug, please report it\n",
> +                avctx->container_crop_left, avctx->container_crop_right, avctx->container_crop_top, avctx->container_crop_bottom,
> +                frame->crop_left, frame->crop_right, frame->crop_top, frame->crop_bottom,
> +                frame->width, frame->height);
> +            frame->crop_left   = 0;
> +            frame->crop_right  = 0;
> +            frame->crop_top    = 0;
> +            frame->crop_bottom = 0;
> +            return 0;
> +        }
> +
> +        frame->crop_top    += avctx->container_crop_top;
> +        frame->crop_left   += avctx->container_crop_left;
> +        frame->crop_bottom += avctx->container_crop_bottom;
> +        frame->crop_right  += avctx->container_crop_right;
> +    }else if (avctx->container_apply_cropping == FF_CONTAINER_CROPPING_OVERWRITE) {
> +
> +        /*check the croppping parameters from container are reasonable and correct*/
> +        if (avctx->container_crop_left >= INT_MAX - avctx->container_crop_right ||
> +            avctx->container_crop_top >= INT_MAX - avctx->container_crop_bottom ||
> +            (avctx->container_crop_left + avctx->container_crop_right) >= frame->width ||
> +            (avctx->container_crop_top + avctx->container_crop_bottom) >= frame->height) {

> +            av_log(avctx, AV_LOG_WARNING,
> +                "Invalid container cropping information set by a demuxer: "
> +                "%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER" "
> +                "(frame size %dx%d). This is a bug, please report it\n",
> +                avctx->container_crop_left, avctx->container_crop_right, avctx->container_crop_top, avctx->container_crop_bottom,
> +                frame->width, frame->height);

The types seems mismatching

thx

[...]
Dmitrii Ovchinnikov Nov. 23, 2022, 2:44 p.m. UTC | #2 
>>The types seems mismatching

The reason for the mismatch of types is that I left the
int type according to the comments :
"
James Almer
IMO the AVFrame ones should have not been size_t to begin with, not just
because the actual dimensions you'll apply them to are int, but because
these fields are not arch dependent or meant for the size of some object
in memory.
"

 It seems that performing casts would not be a good idea.
 Should I make crop fields size_t type as in AVFrame?
 Or is there a more correct way?
Michael Niedermayer Nov. 24, 2022, 11:07 p.m. UTC | #3 
On Wed, Nov 23, 2022 at 03:44:25PM +0100, Dmitrii Ovchinnikov wrote:
> >>The types seems mismatching
> 
> The reason for the mismatch of types is that I left the
> int type according to the comments :
> "
> James Almer
> IMO the AVFrame ones should have not been size_t to begin with, not just
> because the actual dimensions you'll apply them to are int, but because
> these fields are not arch dependent or meant for the size of some object
> in memory.
> "
> 
>  It seems that performing casts would not be a good idea.
>  Should I make crop fields size_t type as in AVFrame?
>  Or is there a more correct way?

the type specified in the format string has to match the types passed
in the argument to av_log
"%"SIZE_SPECIFIER" + size_t
or
%d + int

otherwise you have undefined behavior

[...]
diff mbox series

Patch

diff --git a/libavcodec/codec_par.c b/libavcodec/codec_par.c
index abda649aa8..9738402434 100644
--- a/libavcodec/codec_par.c
+++ b/libavcodec/codec_par.c
@@ -115,17 +115,21 @@  int avcodec_parameters_from_context(AVCodecParameters *par,
 
     switch (par->codec_type) {
     case AVMEDIA_TYPE_VIDEO:
-        par->format              = codec->pix_fmt;
-        par->width               = codec->width;
-        par->height              = codec->height;
-        par->field_order         = codec->field_order;
-        par->color_range         = codec->color_range;
-        par->color_primaries     = codec->color_primaries;
-        par->color_trc           = codec->color_trc;
-        par->color_space         = codec->colorspace;
-        par->chroma_location     = codec->chroma_sample_location;
-        par->sample_aspect_ratio = codec->sample_aspect_ratio;
-        par->video_delay         = codec->has_b_frames;
+        par->format                = codec->pix_fmt;
+        par->width                 = codec->width;
+        par->height                = codec->height;
+        par->container_crop_top    = codec->container_crop_top;
+        par->container_crop_left   = codec->container_crop_left;
+        par->container_crop_bottom = codec->container_crop_bottom;
+        par->container_crop_right  = codec->container_crop_right;
+        par->field_order           = codec->field_order;
+        par->color_range           = codec->color_range;
+        par->color_primaries       = codec->color_primaries;
+        par->color_trc             = codec->color_trc;
+        par->color_space           = codec->colorspace;
+        par->chroma_location       = codec->chroma_sample_location;
+        par->sample_aspect_ratio   = codec->sample_aspect_ratio;
+        par->video_delay           = codec->has_b_frames;
         break;
     case AVMEDIA_TYPE_AUDIO:
         par->format           = codec->sample_fmt;
@@ -199,6 +203,10 @@  int avcodec_parameters_to_context(AVCodecContext *codec,
         codec->pix_fmt                = par->format;
         codec->width                  = par->width;
         codec->height                 = par->height;
+        codec->container_crop_top     = par->container_crop_top;
+        codec->container_crop_left    = par->container_crop_left;
+        codec->container_crop_bottom  = par->container_crop_bottom;
+        codec->container_crop_right   = par->container_crop_right;
         codec->field_order            = par->field_order;
         codec->color_range            = par->color_range;
         codec->color_primaries        = par->color_primaries;
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 6be2d3d6ed..9e44fcb293 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -693,6 +693,60 @@  static int apply_cropping(AVCodecContext *avctx, AVFrame *frame)
     if (!avctx->apply_cropping)
         return 0;
 
+    if (avctx->container_apply_cropping == FF_CONTAINER_CROPPING_ADDITION)
+    {
+        /*check if container parameter and elementary streaming cropping parameters are safe for applying  */
+        if (avctx->container_crop_left + frame->crop_left>= INT_MAX - (avctx->container_crop_right + frame->crop_right) ||
+            avctx->container_crop_top + frame->crop_top >= INT_MAX - (avctx->container_crop_bottom + frame->crop_bottom) ||
+            (avctx->container_crop_left + frame->crop_left + avctx->container_crop_right + frame->crop_right) >= frame->width ||
+            (avctx->container_crop_top + frame->crop_top + avctx->container_crop_bottom + frame->crop_bottom) >= frame->height) {
+            av_log(avctx, AV_LOG_WARNING,
+                "Apply container  and elementary stream corpping parametes error: "
+                "container cropping parameters"
+                "%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER" "
+                "elementary stream croping paramters"
+                "%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER" "
+                "(frame size %dx%d). This is a bug, please report it\n",
+                avctx->container_crop_left, avctx->container_crop_right, avctx->container_crop_top, avctx->container_crop_bottom,
+                frame->crop_left, frame->crop_right, frame->crop_top, frame->crop_bottom,
+                frame->width, frame->height);
+            frame->crop_left   = 0;
+            frame->crop_right  = 0;
+            frame->crop_top    = 0;
+            frame->crop_bottom = 0;
+            return 0;
+        }
+
+        frame->crop_top    += avctx->container_crop_top;
+        frame->crop_left   += avctx->container_crop_left;
+        frame->crop_bottom += avctx->container_crop_bottom;
+        frame->crop_right  += avctx->container_crop_right;
+    }else if (avctx->container_apply_cropping == FF_CONTAINER_CROPPING_OVERWRITE) {
+
+        /*check the croppping parameters from container are reasonable and correct*/
+        if (avctx->container_crop_left >= INT_MAX - avctx->container_crop_right ||
+            avctx->container_crop_top >= INT_MAX - avctx->container_crop_bottom ||
+            (avctx->container_crop_left + avctx->container_crop_right) >= frame->width ||
+            (avctx->container_crop_top + avctx->container_crop_bottom) >= frame->height) {
+            av_log(avctx, AV_LOG_WARNING,
+                "Invalid container cropping information set by a demuxer: "
+                "%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER"/%"SIZE_SPECIFIER" "
+                "(frame size %dx%d). This is a bug, please report it\n",
+                avctx->container_crop_left, avctx->container_crop_right, avctx->container_crop_top, avctx->container_crop_bottom,
+                frame->width, frame->height);
+            frame->crop_left = 0;
+            frame->crop_right = 0;
+            frame->crop_top = 0;
+            frame->crop_bottom = 0;
+            return 0;
+        }
+
+        frame->crop_top    = avctx->container_crop_top;
+        frame->crop_left   = avctx->container_crop_left;
+        frame->crop_bottom = avctx->container_crop_bottom;
+        frame->crop_right  = avctx->container_crop_right;
+    }
+
     return av_frame_apply_cropping(frame, avctx->flags & AV_CODEC_FLAG_UNALIGNED ?
                                           AV_FRAME_CROP_UNALIGNED : 0);
 }