[FFmpeg-devel] [PATCH 1/5] avcodec/packet: add a decoded frame cropping side data type

[FFmpeg-devel] [PATCH 1/5] avcodec/packet: add a decoded frame cropping side data type

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/packet.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/libavcodec/packet.h b/libavcodec/packet.h
index f28e7e7011..f93f2b6f10 100644
--- a/libavcodec/packet.h
+++ b/libavcodec/packet.h
@@ -299,6 +299,20 @@ enum AVPacketSideDataType {
      */
     AV_PKT_DATA_DYNAMIC_HDR10_PLUS,
 
+    /**
+     * The number of pixels to discard from the
+     * top/bottom/left/right border of the decoded frame to obtain the sub-rectangle
+     * intended for presentation.
+     *
+     * @code
+     * u32le crop_top
+     * u32le crop_bottom
+     * u32le crop_left
+     * u32le crop_right
+     * @endcode
+     */
+    AV_PKT_DATA_FRAME_CROPPING,
+
     /**
      * The number of side data types.
      * This is not part of the public API/ABI in the sense that it may
-- 
2.41.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] [PATCH 2/5] avformat/dump: print Frame Cropping side data info

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/dump.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/libavformat/dump.c b/libavformat/dump.c
index d31e4c2ec6..b35bbd185f 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -427,6 +427,23 @@ static void dump_s12m_timecode(void *ctx, const AVStream *st, const AVPacketSide
     }
 }
 
+static void dump_cropping(void *ctx, const AVPacketSideData *sd)
+{
+    uint32_t top, bottom, left, right;
+
+    if (sd->size != sizeof(uint32_t) * 4) {
+        av_log(ctx, AV_LOG_ERROR, "invalid data\n");
+        return;
+    }
+
+    top    = AV_RL32(sd->data +  0);
+    bottom = AV_RL32(sd->data +  4);
+    left   = AV_RL32(sd->data +  8);
+    right  = AV_RL32(sd->data + 12);
+
+    av_log(ctx, AV_LOG_INFO, "%d/%d/%d/%d", left, right, top, bottom);
+}
+
 static void dump_sidedata(void *ctx, const AVStream *st, const char *indent)
 {
     int i;
@@ -497,6 +514,10 @@ static void dump_sidedata(void *ctx, const AVStream *st, const char *indent)
             av_log(ctx, AV_LOG_INFO, "SMPTE ST 12-1:2014: ");
             dump_s12m_timecode(ctx, st, sd);
             break;
+        case AV_PKT_DATA_FRAME_CROPPING:
+            av_log(ctx, AV_LOG_INFO, "Frame cropping: ");
+            dump_cropping(ctx, sd);
+            break;
         default:
             av_log(ctx, AV_LOG_INFO, "unknown side data type %d "
                    "(%"SIZE_SPECIFIER" bytes)", sd->type, sd->size);
-- 
2.41.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] [PATCH 3/5] avformat/matroskadec: export cropping values

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/matroskadec.c | 49 +++++++++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 49950956b6..46459b3dc8 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -211,7 +211,13 @@ typedef struct MatroskaTrackVideo {
     uint64_t display_height;
     uint64_t pixel_width;
     uint64_t pixel_height;
+    uint64_t cropped_width;
+    uint64_t cropped_height;
     EbmlBin  color_space;
+    uint64_t pixel_cropt;
+    uint64_t pixel_cropl;
+    uint64_t pixel_cropb;
+    uint64_t pixel_cropr;
     uint64_t display_unit;
     uint64_t interlaced;
     uint64_t field_order;
@@ -525,10 +531,10 @@ static EbmlSyntax matroska_track_video[] = {
     { MATROSKA_ID_VIDEOALPHAMODE,      EBML_UINT,  0, 0, offsetof(MatroskaTrackVideo, alpha_mode), { .u = 0 } },
     { MATROSKA_ID_VIDEOCOLOR,          EBML_NEST,  0, sizeof(MatroskaTrackVideoColor), offsetof(MatroskaTrackVideo, color), { .n = matroska_track_video_color } },
     { MATROSKA_ID_VIDEOPROJECTION,     EBML_NEST,  0, 0, offsetof(MatroskaTrackVideo, projection), { .n = matroska_track_video_projection } },
-    { MATROSKA_ID_VIDEOPIXELCROPB,     EBML_NONE },
-    { MATROSKA_ID_VIDEOPIXELCROPT,     EBML_NONE },
-    { MATROSKA_ID_VIDEOPIXELCROPL,     EBML_NONE },
-    { MATROSKA_ID_VIDEOPIXELCROPR,     EBML_NONE },
+    { MATROSKA_ID_VIDEOPIXELCROPB,     EBML_UINT,  0, 0, offsetof(MatroskaTrackVideo, pixel_cropb), {.u = 0 } },
+    { MATROSKA_ID_VIDEOPIXELCROPT,     EBML_UINT,  0, 0, offsetof(MatroskaTrackVideo, pixel_cropt), {.u = 0 } },
+    { MATROSKA_ID_VIDEOPIXELCROPL,     EBML_UINT,  0, 0, offsetof(MatroskaTrackVideo, pixel_cropl), {.u = 0 } },
+    { MATROSKA_ID_VIDEOPIXELCROPR,     EBML_UINT,  0, 0, offsetof(MatroskaTrackVideo, pixel_cropr), {.u = 0 } },
     { MATROSKA_ID_VIDEODISPLAYUNIT,    EBML_UINT,  0, 0, offsetof(MatroskaTrackVideo, display_unit), { .u= MATROSKA_VIDEO_DISPLAYUNIT_PIXELS } },
     { MATROSKA_ID_VIDEOFLAGINTERLACED, EBML_UINT,  0, 0, offsetof(MatroskaTrackVideo, interlaced),  { .u = MATROSKA_VIDEO_INTERLACE_FLAG_UNDETERMINED } },
     { MATROSKA_ID_VIDEOFIELDORDER,     EBML_UINT,  0, 0, offsetof(MatroskaTrackVideo, field_order), { .u = MATROSKA_VIDEO_FIELDORDER_UNDETERMINED } },
@@ -2527,10 +2533,21 @@ static int matroska_parse_tracks(AVFormatContext *s)
                     track->default_duration = default_duration;
                 }
             }
+            track->video.cropped_width  = track->video.pixel_width;
+            track->video.cropped_height = track->video.pixel_height;
+            if (track->video.display_unit == 0) {
+                if (track->video.pixel_cropl >= INT_MAX - track->video.pixel_cropr ||
+                    track->video.pixel_cropt >= INT_MAX - track->video.pixel_cropb ||
+                    (track->video.pixel_cropl + track->video.pixel_cropr) >= track->video.pixel_width ||
+                    (track->video.pixel_cropt + track->video.pixel_cropb) >= track->video.pixel_height)
+                    return AVERROR_INVALIDDATA;
+                track->video.cropped_width  -= track->video.pixel_cropl + track->video.pixel_cropr;
+                track->video.cropped_height -= track->video.pixel_cropt + track->video.pixel_cropb;
+            }
             if (track->video.display_width == -1)
-                track->video.display_width = track->video.pixel_width;
+                track->video.display_width = track->video.cropped_width;
             if (track->video.display_height == -1)
-                track->video.display_height = track->video.pixel_height;
+                track->video.display_height = track->video.cropped_height;
             if (track->video.color_space.size == 4)
                 fourcc = AV_RL32(track->video.color_space.data);
         } else if (track->type == MATROSKA_TRACK_TYPE_AUDIO) {
@@ -2955,14 +2972,26 @@ static int matroska_parse_tracks(AVFormatContext *s)
 
             if (track->video.display_unit < MATROSKA_VIDEO_DISPLAYUNIT_UNKNOWN) {
                 if (track->video.display_width && track->video.display_height &&
-                    st->codecpar->height  < INT64_MAX / track->video.display_width  / display_width_mul &&
-                    st->codecpar->width   < INT64_MAX / track->video.display_height / display_height_mul)
+                    track->video.cropped_height < INT64_MAX / track->video.display_width  / display_width_mul &&
+                    track->video.cropped_width  < INT64_MAX / track->video.display_height / display_height_mul)
                     av_reduce(&st->sample_aspect_ratio.num,
                               &st->sample_aspect_ratio.den,
-                              st->codecpar->height * track->video.display_width  * display_width_mul,
-                              st->codecpar->width  * track->video.display_height * display_height_mul,
+                              track->video.cropped_height * track->video.display_width  * display_width_mul,
+                              track->video.cropped_width  * track->video.display_height * display_height_mul,
                               INT_MAX);
             }
+            if (track->video.cropped_width  != track->video.pixel_width ||
+                track->video.cropped_height != track->video.pixel_height) {
+                uint8_t *cropping = av_stream_new_side_data(st, AV_PKT_DATA_FRAME_CROPPING, sizeof(uint32_t) * 4);
+                if (!cropping)
+                    return AVERROR(ENOMEM);
+
+                bytestream_put_le32(&cropping, track->video.pixel_cropt);
+                bytestream_put_le32(&cropping, track->video.pixel_cropb);
+                bytestream_put_le32(&cropping, track->video.pixel_cropl);
+                bytestream_put_le32(&cropping, track->video.pixel_cropr);
+            }
+
             if (st->codecpar->codec_id != AV_CODEC_ID_HEVC)
                 sti->need_parsing = AVSTREAM_PARSE_HEADERS;
 
-- 
2.41.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] [PATCH 4/5] avformat/matroskaenc: support writing cropping values

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/matroskaenc.c | 45 +++++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 41e13b273d..7bfe7b002a 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1654,8 +1654,11 @@ static int mkv_write_track_video(AVFormatContext *s, MatroskaMuxContext *mkv,
     const AVDictionaryEntry *tag;
     int display_width_div = 1, display_height_div = 1;
     uint8_t color_space[4], projection_private[20];
+    uint8_t *cropping;
     EBML_WRITER(MAX_FIELD_ORDER_ELEMS + MAX_STEREO_MODE_ELEMS      +
                 MAX_VIDEO_COLOR_ELEMS + MAX_VIDEO_PROJECTION_ELEMS + 8);
+    int cropped_width = par->width, cropped_height = par->height;
+    size_t cropped_size;
     int ret;
 
     ebml_writer_open_master(&writer, MATROSKA_ID_TRACKVIDEO);
@@ -1679,25 +1682,53 @@ static int mkv_write_track_video(AVFormatContext *s, MatroskaMuxContext *mkv,
         ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEOALPHAMODE, 1);
     }
 
+    cropping = av_stream_get_side_data(st, AV_PKT_DATA_FRAME_CROPPING, &cropped_size);
+    if (cropping) {
+        uint32_t top, bottom, left, right;
+        if (cropped_size != sizeof(uint32_t) * 4)
+            return AVERROR_INVALIDDATA;
+
+        top    = AV_RL32(cropping +  0);
+        bottom = AV_RL32(cropping +  4);
+        left   = AV_RL32(cropping +  8);
+        right  = AV_RL32(cropping + 12);
+
+        if (left >= INT_MAX - right ||
+            top >= INT_MAX - bottom ||
+            (left + right) >= par->width ||
+            (top + bottom) >= par->height) {
+            av_log(s, AV_LOG_ERROR, "Invalid cropping dimensions in stream side data\n");
+            return AVERROR(EINVAL);
+        }
+
+        ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEOPIXELCROPB, bottom);
+        ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEOPIXELCROPT, top);
+        ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEOPIXELCROPL, left);
+        ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEOPIXELCROPR, right);
+
+        cropped_width  -= left + right;
+        cropped_height -= top + bottom;
+    }
+
     // write DisplayWidth and DisplayHeight, they contain the size of
     // a single source view and/or the display aspect ratio
     if (st->sample_aspect_ratio.num) {
-        int64_t d_width = av_rescale(par->width, st->sample_aspect_ratio.num, st->sample_aspect_ratio.den);
+        int64_t d_width = av_rescale(cropped_width, st->sample_aspect_ratio.num, st->sample_aspect_ratio.den);
         if (d_width > INT_MAX) {
             av_log(s, AV_LOG_ERROR, "Overflow in display width\n");
             return AVERROR(EINVAL);
         }
-        if (d_width != par->width || display_width_div != 1 || display_height_div != 1) {
+        if (d_width != cropped_width || display_width_div != 1 || display_height_div != 1) {
             if (IS_WEBM(mkv) || display_width_div != 1 || display_height_div != 1) {
                 ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEODISPLAYWIDTH,
                                      d_width / display_width_div);
                 ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEODISPLAYHEIGHT,
-                                     par->height / display_height_div);
+                                     cropped_height / display_height_div);
             } else {
                 AVRational display_aspect_ratio;
                 av_reduce(&display_aspect_ratio.num, &display_aspect_ratio.den,
-                            par->width  * (int64_t)st->sample_aspect_ratio.num,
-                            par->height * (int64_t)st->sample_aspect_ratio.den,
+                            cropped_width  * (int64_t)st->sample_aspect_ratio.num,
+                            cropped_height * (int64_t)st->sample_aspect_ratio.den,
                             1024 * 1024);
                 ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEODISPLAYWIDTH,
                                      display_aspect_ratio.num);
@@ -1709,9 +1740,9 @@ static int mkv_write_track_video(AVFormatContext *s, MatroskaMuxContext *mkv,
         }
     } else if (display_width_div != 1 || display_height_div != 1) {
         ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEODISPLAYWIDTH,
-                             par->width / display_width_div);
+                             cropped_width / display_width_div);
         ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEODISPLAYHEIGHT,
-                             par->height / display_height_div);
+                             cropped_height / display_height_div);
     } else if (!IS_WEBM(mkv))
         ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEODISPLAYUNIT,
                              MATROSKA_VIDEO_DISPLAYUNIT_UNKNOWN);
-- 
2.41.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] [PATCH 5/5] fftools/ffmpeg: support applying container level cropping

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
 fftools/ffmpeg.h       |  3 +++
 fftools/ffmpeg_dec.c   | 33 +++++++++++++++++++++++++++++++++
 fftools/ffmpeg_demux.c |  6 ++++++
 fftools/ffmpeg_enc.c   |  6 ++++--
 fftools/ffmpeg_opt.c   |  3 +++
 5 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index f45ddf33b2..dc019b9c20 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -156,6 +156,8 @@ typedef struct OptionsContext {
     int        nb_hwaccel_output_formats;
     SpecifierOpt *autorotate;
     int        nb_autorotate;
+    SpecifierOpt *apply_cropping;
+    int        nb_apply_cropping;
 
     /* output options */
     StreamMap *stream_maps;
@@ -350,6 +352,7 @@ typedef struct InputStream {
     int top_field_first;
 
     int autorotate;
+    int apply_cropping;
 
     int fix_sub_duration;
 
diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
index 5c1b8888e9..5b810f7588 100644
--- a/fftools/ffmpeg_dec.c
+++ b/fftools/ffmpeg_dec.c
@@ -19,6 +19,7 @@
 #include "libavutil/avassert.h"
 #include "libavutil/dict.h"
 #include "libavutil/error.h"
+#include "libavutil/intreadwrite.h"
 #include "libavutil/log.h"
 #include "libavutil/pixdesc.h"
 #include "libavutil/pixfmt.h"
@@ -325,6 +326,38 @@ static int video_frame_process(InputStream *ist, AVFrame *frame)
             ist->dec_ctx->pix_fmt);
     }
 
+    if (ist->apply_cropping) {
+        size_t cropping_size;
+        uint8_t *cropping = av_stream_get_side_data(ist->st, AV_PKT_DATA_FRAME_CROPPING, &cropping_size);
+        if (cropping && cropping_size == sizeof(uint32_t) * 4) {
+            uint32_t top    = AV_RL32(cropping +  0);
+            uint32_t bottom = AV_RL32(cropping +  4);
+            uint32_t left   = AV_RL32(cropping +  8);
+            uint32_t right  = AV_RL32(cropping + 12);
+            int err;
+
+            if (top    < INT_MAX - frame->crop_top    &&
+                bottom < INT_MAX - frame->crop_bottom &&
+                left   < INT_MAX - frame->crop_left   &&
+                right  < INT_MAX - frame->crop_right) {
+
+                frame->crop_top    += top;
+                frame->crop_bottom += bottom;
+                frame->crop_left   += left;
+                frame->crop_right  += right;
+
+                err = av_frame_apply_cropping(frame, ist->dec_ctx->flags & AV_CODEC_FLAG_UNALIGNED ?
+                                          AV_FRAME_CROP_UNALIGNED : 0);
+                if (err < 0)
+                    return err;
+            } else
+                av_log(ist->dec_ctx, AV_LOG_WARNING,
+                       "Invalid cropping information set through side data: %d/%d/%d/%d "
+                       "(frame size %dx%d). Ignoring\n",
+                       left, right, top, bottom, frame->width, frame->height);
+        }
+    }
+
     if(ist->top_field_first>=0)
         frame->flags |= AV_FRAME_FLAG_TOP_FIELD_FIRST;
 
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 9e7f13a2b4..09d040b626 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -48,6 +48,7 @@ static const char *const opt_name_hwaccels[]                  = {"hwaccel", NULL
 static const char *const opt_name_hwaccel_devices[]           = {"hwaccel_device", NULL};
 static const char *const opt_name_hwaccel_output_formats[]    = {"hwaccel_output_format", NULL};
 static const char *const opt_name_autorotate[]                = {"autorotate", NULL};
+static const char *const opt_name_apply_cropping[]            = {"apply_cropping", NULL};
 static const char *const opt_name_display_rotations[]         = {"display_rotation", NULL};
 static const char *const opt_name_display_hflips[]            = {"display_hflip", NULL};
 static const char *const opt_name_display_vflips[]            = {"display_vflip", NULL};
@@ -1068,6 +1069,11 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
     ist->autorotate = 1;
     MATCH_PER_STREAM_OPT(autorotate, i, ist->autorotate, ic, st);
 
+    ist->apply_cropping = 1;
+    MATCH_PER_STREAM_OPT(apply_cropping, i, ist->apply_cropping, ic, st);
+
+    av_dict_set_int(&o->g->codec_opts, "apply_cropping", ist->apply_cropping, 0);
+
     MATCH_PER_STREAM_OPT(codec_tags, str, codec_tag, ic, st);
     if (codec_tag) {
         uint32_t tag = strtol(codec_tag, &next, 0);
diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index 8b750de4e5..3cf29c8b2c 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -441,14 +441,16 @@ int enc_open(OutputStream *ost, AVFrame *frame)
         int i;
         for (i = 0; i < ist->st->nb_side_data; i++) {
             AVPacketSideData *sd = &ist->st->side_data[i];
-            if (sd->type != AV_PKT_DATA_CPB_PROPERTIES) {
+            if (sd->type == AV_PKT_DATA_CPB_PROPERTIES)
+                continue;
+            if (ist->apply_cropping && sd->type == AV_PKT_DATA_FRAME_CROPPING)
+                continue;
                 uint8_t *dst = av_stream_new_side_data(ost->st, sd->type, sd->size);
                 if (!dst)
                     return AVERROR(ENOMEM);
                 memcpy(dst, sd->data, sd->size);
                 if (ist->autorotate && sd->type == AV_PKT_DATA_DISPLAYMATRIX)
                     av_display_rotation_set((int32_t *)dst, 0);
-            }
         }
     }
 
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 300f042660..fe61a73643 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1677,6 +1677,9 @@ const OptionDef options[] = {
     { "autorotate",       HAS_ARG | OPT_BOOL | OPT_SPEC |
                           OPT_EXPERT | OPT_INPUT,                                { .off = OFFSET(autorotate) },
         "automatically insert correct rotate filters" },
+    { "apply_cropping",   HAS_ARG | OPT_BOOL | OPT_SPEC |
+                          OPT_EXPERT | OPT_INPUT,                                { .off = OFFSET(apply_cropping) },
+        "Apply frame cropping instead of exporting it" },
     { "autoscale",        HAS_ARG | OPT_BOOL | OPT_SPEC |
                           OPT_EXPERT | OPT_OUTPUT,                               { .off = OFFSET(autoscale) },
         "automatically insert a scale filter at the end of the filter graph" },
-- 
2.41.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".

Re: [FFmpeg-devel] [PATCH 5/5] fftools/ffmpeg: support applying container level cropping

[Anton Khirnov]

Quoting James Almer (2023-07-20 00:20:43)
> diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
> index 8b750de4e5..3cf29c8b2c 100644
> --- a/fftools/ffmpeg_enc.c
> +++ b/fftools/ffmpeg_enc.c
> @@ -441,14 +441,16 @@ int enc_open(OutputStream *ost, AVFrame *frame)
>          int i;
>          for (i = 0; i < ist->st->nb_side_data; i++) {
>              AVPacketSideData *sd = &ist->st->side_data[i];
> -            if (sd->type != AV_PKT_DATA_CPB_PROPERTIES) {
> +            if (sd->type == AV_PKT_DATA_CPB_PROPERTIES)
> +                continue;
> +            if (ist->apply_cropping && sd->type == AV_PKT_DATA_FRAME_CROPPING)
> +                continue;

I'm very much not a fan of the encoder doing anything based on decoder
options.

I know the code below already does the same thing, but I'd like to get
rid of it rather than add to it.

-- 
Anton Khirnov
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 5/5] fftools/ffmpeg: support applying container level cropping

[James Almer]

On 7/20/2023 4:08 PM, Anton Khirnov wrote:
> Quoting James Almer (2023-07-20 00:20:43)
>> diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
>> index 8b750de4e5..3cf29c8b2c 100644
>> --- a/fftools/ffmpeg_enc.c
>> +++ b/fftools/ffmpeg_enc.c
>> @@ -441,14 +441,16 @@ int enc_open(OutputStream *ost, AVFrame *frame)
>>           int i;
>>           for (i = 0; i < ist->st->nb_side_data; i++) {
>>               AVPacketSideData *sd = &ist->st->side_data[i];
>> -            if (sd->type != AV_PKT_DATA_CPB_PROPERTIES) {
>> +            if (sd->type == AV_PKT_DATA_CPB_PROPERTIES)
>> +                continue;
>> +            if (ist->apply_cropping && sd->type == AV_PKT_DATA_FRAME_CROPPING)
>> +                continue;
> 
> I'm very much not a fan of the encoder doing anything based on decoder
> options.

Right now, all input stream side data (save for CPB) is copied to the 
output stream. Without this chunk, the frame cropping side data will be 
copied regardless of it having been applied or not at the decoding level.
I don't know how else to prevent that. Maybe removing the side data from 
the input stream? Although that's pretty ugly.

I have a separate patchset adding packet side data to codecpar and 
avctx, and deprecating AVStream.side_data in favor of it. With that, i 
could maybe use and therefore remove the cropping side data from 
ist->par (which is internal to the CLI) if applied.

> 
> I know the code below already does the same thing, but I'd like to get
> rid of it rather than add to it.
> 
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 5/5] fftools/ffmpeg: support applying container level cropping

[Anton Khirnov]

Quoting James Almer (2023-07-20 21:25:02)
> On 7/20/2023 4:08 PM, Anton Khirnov wrote:
> > Quoting James Almer (2023-07-20 00:20:43)
> >> diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
> >> index 8b750de4e5..3cf29c8b2c 100644
> >> --- a/fftools/ffmpeg_enc.c
> >> +++ b/fftools/ffmpeg_enc.c
> >> @@ -441,14 +441,16 @@ int enc_open(OutputStream *ost, AVFrame *frame)
> >>           int i;
> >>           for (i = 0; i < ist->st->nb_side_data; i++) {
> >>               AVPacketSideData *sd = &ist->st->side_data[i];
> >> -            if (sd->type != AV_PKT_DATA_CPB_PROPERTIES) {
> >> +            if (sd->type == AV_PKT_DATA_CPB_PROPERTIES)
> >> +                continue;
> >> +            if (ist->apply_cropping && sd->type == AV_PKT_DATA_FRAME_CROPPING)
> >> +                continue;
> > 
> > I'm very much not a fan of the encoder doing anything based on decoder
> > options.
> 
> Right now, all input stream side data (save for CPB) is copied to the 
> output stream.

I think it's wrong for transcoding. Side data should be propagated
through the decoder and the filtergraph and then be processed by the
encoder. Just blindly copying whatever is in the input is bound to
produce inaccurate information.

-- 
Anton Khirnov
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 5/5] fftools/ffmpeg: support applying container level cropping

[James Almer]

On 7/20/2023 4:31 PM, Anton Khirnov wrote:
> Quoting James Almer (2023-07-20 21:25:02)
>> On 7/20/2023 4:08 PM, Anton Khirnov wrote:
>>> Quoting James Almer (2023-07-20 00:20:43)
>>>> diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
>>>> index 8b750de4e5..3cf29c8b2c 100644
>>>> --- a/fftools/ffmpeg_enc.c
>>>> +++ b/fftools/ffmpeg_enc.c
>>>> @@ -441,14 +441,16 @@ int enc_open(OutputStream *ost, AVFrame *frame)
>>>>            int i;
>>>>            for (i = 0; i < ist->st->nb_side_data; i++) {
>>>>                AVPacketSideData *sd = &ist->st->side_data[i];
>>>> -            if (sd->type != AV_PKT_DATA_CPB_PROPERTIES) {
>>>> +            if (sd->type == AV_PKT_DATA_CPB_PROPERTIES)
>>>> +                continue;
>>>> +            if (ist->apply_cropping && sd->type == AV_PKT_DATA_FRAME_CROPPING)
>>>> +                continue;
>>>
>>> I'm very much not a fan of the encoder doing anything based on decoder
>>> options.
>>
>> Right now, all input stream side data (save for CPB) is copied to the
>> output stream.
> 
> I think it's wrong for transcoding. Side data should be propagated
> through the decoder

My other set will introduce this, so at least the first step towards 
this will be done. I'll send it in a few.

> and the filtergraph and then be processed by the
> encoder.

It however doesn't touch lavfi.

> Just blindly copying whatever is in the input is bound to
> produce inaccurate information.

Agree, but that's outside the scope of this set. I don't think lavfi can 
even take global side data right now. Only on a frame basis.
_______________________________________________
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] [PATCH v2 5/5] fftools/ffmpeg: support applying container level cropping

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
Now inserting a filter into the graph.

 fftools/ffmpeg.h        |  3 +++
 fftools/ffmpeg_demux.c  |  6 ++++++
 fftools/ffmpeg_enc.c    | 19 +++++++++++--------
 fftools/ffmpeg_filter.c | 22 ++++++++++++++++++++++
 fftools/ffmpeg_opt.c    |  3 +++
 5 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index d53181e427..bb6d510f10 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -154,6 +154,8 @@ typedef struct OptionsContext {
     int        nb_hwaccel_output_formats;
     SpecifierOpt *autorotate;
     int        nb_autorotate;
+    SpecifierOpt *apply_cropping;
+    int        nb_apply_cropping;
 
     /* output options */
     StreamMap *stream_maps;
@@ -347,6 +349,7 @@ typedef struct InputStream {
     int top_field_first;
 
     int autorotate;
+    int apply_cropping;
 
     int fix_sub_duration;
 
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 48edbd7f6b..1209cf2046 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -48,6 +48,7 @@ static const char *const opt_name_hwaccels[]                  = {"hwaccel", NULL
 static const char *const opt_name_hwaccel_devices[]           = {"hwaccel_device", NULL};
 static const char *const opt_name_hwaccel_output_formats[]    = {"hwaccel_output_format", NULL};
 static const char *const opt_name_autorotate[]                = {"autorotate", NULL};
+static const char *const opt_name_apply_cropping[]            = {"apply_cropping", NULL};
 static const char *const opt_name_display_rotations[]         = {"display_rotation", NULL};
 static const char *const opt_name_display_hflips[]            = {"display_hflip", NULL};
 static const char *const opt_name_display_vflips[]            = {"display_vflip", NULL};
@@ -1085,6 +1086,11 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
     ist->autorotate = 1;
     MATCH_PER_STREAM_OPT(autorotate, i, ist->autorotate, ic, st);
 
+    ist->apply_cropping = 1;
+    MATCH_PER_STREAM_OPT(apply_cropping, i, ist->apply_cropping, ic, st);
+
+    av_dict_set_int(&o->g->codec_opts, "apply_cropping", ist->apply_cropping, 0);
+
     MATCH_PER_STREAM_OPT(codec_tags, str, codec_tag, ic, st);
     if (codec_tag) {
         uint32_t tag = strtol(codec_tag, &next, 0);
diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index 96424272bf..cd30986344 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -448,14 +448,17 @@ int enc_open(OutputStream *ost, AVFrame *frame)
         int i;
         for (i = 0; i < ist->st->nb_side_data; i++) {
             AVPacketSideData *sd = &ist->st->side_data[i];
-            if (sd->type != AV_PKT_DATA_CPB_PROPERTIES) {
-                uint8_t *dst = av_stream_new_side_data(ost->st, sd->type, sd->size);
-                if (!dst)
-                    return AVERROR(ENOMEM);
-                memcpy(dst, sd->data, sd->size);
-                if (ist->autorotate && sd->type == AV_PKT_DATA_DISPLAYMATRIX)
-                    av_display_rotation_set((int32_t *)dst, 0);
-            }
+            uint8_t *dst;
+            if (sd->type == AV_PKT_DATA_CPB_PROPERTIES)
+                continue;
+            if (ist->apply_cropping && sd->type == AV_PKT_DATA_FRAME_CROPPING)
+                continue;
+            dst = av_stream_new_side_data(ost->st, sd->type, sd->size);
+            if (!dst)
+                return AVERROR(ENOMEM);
+            memcpy(dst, sd->data, sd->size);
+            if (ist->autorotate && sd->type == AV_PKT_DATA_DISPLAYMATRIX)
+                av_display_rotation_set((int32_t *)dst, 0);
         }
     }
 
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 925b5116cc..8cadb4732c 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -31,6 +31,7 @@
 #include "libavutil/bprint.h"
 #include "libavutil/channel_layout.h"
 #include "libavutil/display.h"
+#include "libavutil/intreadwrite.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
 #include "libavutil/pixfmt.h"
@@ -1418,6 +1419,27 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
             return ret;
     }
 
+    if (ist->apply_cropping && !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
+        size_t cropping_size;
+        uint8_t *cropping = av_stream_get_side_data(ist->st, AV_PKT_DATA_FRAME_CROPPING, &cropping_size);
+
+        if (cropping && cropping_size == sizeof(uint32_t) * 4) {
+            char crop_buf[64];
+            int top    = AV_RL32(cropping +  0);
+            int bottom = AV_RL32(cropping +  4);
+            int left   = AV_RL32(cropping +  8);
+            int right  = AV_RL32(cropping + 12);
+
+            if (top < 0 || bottom < 0 || left < 0 || right < 0)
+                return AVERROR(EINVAL);
+
+            snprintf(crop_buf, sizeof(crop_buf), "w=iw-%d-%d:h=ih-%d-%d", left, right, top, bottom);
+            ret = insert_filter(&last_filter, &pad_idx, "crop", crop_buf);
+            if (ret < 0)
+                return ret;
+        }
+    }
+
     snprintf(name, sizeof(name), "trim_in_%d_%d",
              ist->file_index, ist->index);
     if (copy_ts) {
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index dc6044120a..30bb919cb6 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1733,6 +1733,9 @@ const OptionDef options[] = {
     { "autorotate",       HAS_ARG | OPT_BOOL | OPT_SPEC |
                           OPT_EXPERT | OPT_INPUT,                                { .off = OFFSET(autorotate) },
         "automatically insert correct rotate filters" },
+    { "apply_cropping",   HAS_ARG | OPT_BOOL | OPT_SPEC |
+                          OPT_EXPERT | OPT_INPUT,                                { .off = OFFSET(apply_cropping) },
+        "Apply frame cropping instead of exporting it" },
     { "autoscale",        HAS_ARG | OPT_BOOL | OPT_SPEC |
                           OPT_EXPERT | OPT_OUTPUT,                               { .off = OFFSET(autoscale) },
         "automatically insert a scale filter at the end of the filter graph" },
-- 
2.41.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".

Re: [FFmpeg-devel] [PATCH v2 5/5] fftools/ffmpeg: support applying container level cropping

[Tomas Härdin]

tis 2023-07-25 klockan 14:09 -0300 skrev James Almer:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Now inserting a filter into the graph.

This looks useful for MXF

> +    { "apply_cropping",   HAS_ARG | OPT_BOOL | OPT_SPEC |
> +                          OPT_EXPERT |
> OPT_INPUT,                                { .off =
> OFFSET(apply_cropping) },
> +        "Apply frame cropping instead of exporting it" },

Hm. Can this be applied automatically for ffplay? When transcoding I
expect the typical use case is to not crop and to carry the metadata
over. MXF -> MOV for example. But when playing I expect one just wants
to see the display rectangle.

/Tomas
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 5/5] fftools/ffmpeg: support applying container level cropping

[James Almer]

On 7/26/2023 6:42 PM, Tomas Härdin wrote:
> tis 2023-07-25 klockan 14:09 -0300 skrev James Almer:
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> Now inserting a filter into the graph.
> 
> This looks useful for MXF
> 
>> +    { "apply_cropping",   HAS_ARG | OPT_BOOL | OPT_SPEC |
>> +                          OPT_EXPERT |
>> OPT_INPUT,                                { .off =
>> OFFSET(apply_cropping) },
>> +        "Apply frame cropping instead of exporting it" },
> 
> Hm. Can this be applied automatically for ffplay? When transcoding I
> expect the typical use case is to not crop and to carry the metadata
> over. MXF -> MOV for example. But when playing I expect one just wants
> to see the display rectangle.

You want it to be disabled by default on ffmpeg but enabled in ffplay?

For the latter, something like:

> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index 5212ad053e..217fc3e45a 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -36,6 +36,7 @@
>  #include "libavutil/eval.h"
>  #include "libavutil/mathematics.h"
>  #include "libavutil/pixdesc.h"
> +#include "libavutil/intreadwrite.h"
>  #include "libavutil/imgutils.h"
>  #include "libavutil/dict.h"
>  #include "libavutil/fifo.h"
> @@ -346,6 +347,7 @@ static const char **vfilters_list = NULL;
>  static int nb_vfilters = 0;
>  static char *afilters = NULL;
>  static int autorotate = 1;
> +static int apply_cropping = 1;
>  static int find_stream_info = 1;
>  static int filter_nbthreads = 0;
> 
> @@ -1922,6 +1924,27 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>          }
>      }
> 
> +    if (apply_cropping) {
> +        size_t cropping_size;
> +        uint8_t *cropping = av_stream_get_side_data(is->video_st, AV_PKT_DATA_FRAME_CROPPING, &cropping_size);
> +
> +        if (cropping && cropping_size == sizeof(uint32_t) * 4) {
> +            char crop_buf[64];
> +            int top    = AV_RL32(cropping +  0);
> +            int bottom = AV_RL32(cropping +  4);
> +            int left   = AV_RL32(cropping +  8);
> +            int right  = AV_RL32(cropping + 12);
> +
> +            if (top < 0 || bottom < 0 || left < 0 || right < 0)  {
> +                ret = AVERROR(EINVAL);
> +                goto fail;
> +            }
> +
> +            snprintf(crop_buf, sizeof(crop_buf), "w=iw-%d-%d:h=ih-%d-%d", left, right, top, bottom);
> +            INSERT_FILT("crop", crop_buf);
> +        }
> +    }
> +
>      if ((ret = configure_filtergraph(graph, vfilters, filt_src, last_filter)) < 0)
>          goto fail;
> 
> @@ -3593,6 +3616,7 @@ static const OptionDef options[] = {
>      { "scodec", HAS_ARG | OPT_STRING | OPT_EXPERT, { &subtitle_codec_name }, "force subtitle decoder", "decoder_name" },
>      { "vcodec", HAS_ARG | OPT_STRING | OPT_EXPERT, {    &video_codec_name }, "force video decoder",    "decoder_name" },
>      { "autorotate", OPT_BOOL, { &autorotate }, "automatically rotate video", "" },
> +    { "apply_cropping", OPT_BOOL, { &apply_cropping }, "apply frame cropping", "" },
>      { "find_stream_info", OPT_BOOL | OPT_INPUT | OPT_EXPERT, { &find_stream_info },
>          "read and decode the streams to fill missing information with heuristics" },
>      { "filter_threads", HAS_ARG | OPT_INT | OPT_EXPERT, { &filter_nbthreads }, "number of filter threads per graph" },

Would do it, but i don't know if this affects the AVCodecContext option 
of the same name too or not (to apply or not bitstream level cropping, 
like the h264 one, which is obviously enabled by default).

To have it disabled on ffmpeg by default, i think the following would 
work (on top of this patch):

> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index 1209cf2046..a37c136cf9 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -1086,10 +1086,11 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>      ist->autorotate = 1;
>      MATCH_PER_STREAM_OPT(autorotate, i, ist->autorotate, ic, st);
> 
> -    ist->apply_cropping = 1;
> +    ist->apply_cropping = -1;
>      MATCH_PER_STREAM_OPT(apply_cropping, i, ist->apply_cropping, ic, st);
> 
> -    av_dict_set_int(&o->g->codec_opts, "apply_cropping", ist->apply_cropping, 0);
> +    if (ist->apply_cropping >= 0)
> +        av_dict_set_int(&o->g->codec_opts, "apply_cropping", ist->apply_cropping, 0);
> 
>      MATCH_PER_STREAM_OPT(codec_tags, str, codec_tag, ic, st);
>      if (codec_tag) {
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index 8cadb4732c..5df52ef718 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -1419,7 +1419,7 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
>              return ret;
>      }
> 
> -    if (ist->apply_cropping && !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
> +    if (ist->apply_cropping > 0 && !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
>          size_t cropping_size;
>          uint8_t *cropping = av_stream_get_side_data(ist->st, AV_PKT_DATA_FRAME_CROPPING, &cropping_size);
> 

And actually work fine with the AVCodecContext option.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 5/5] fftools/ffmpeg: support applying container level cropping

[Tomas Härdin]

ons 2023-07-26 klockan 19:11 -0300 skrev James Almer:
> On 7/26/2023 6:42 PM, Tomas Härdin wrote:
> > tis 2023-07-25 klockan 14:09 -0300 skrev James Almer:
> > > Signed-off-by: James Almer <jamrial@gmail.com>
> > > ---
> > > Now inserting a filter into the graph.
> > 
> > This looks useful for MXF
> > 
> > > +    { "apply_cropping",   HAS_ARG | OPT_BOOL | OPT_SPEC |
> > > +                          OPT_EXPERT |
> > > OPT_INPUT,                                { .off =
> > > OFFSET(apply_cropping) },
> > > +        "Apply frame cropping instead of exporting it" },
> > 
> > Hm. Can this be applied automatically for ffplay? When transcoding
> > I
> > expect the typical use case is to not crop and to carry the
> > metadata
> > over. MXF -> MOV for example. But when playing I expect one just
> > wants
> > to see the display rectangle.
> 
> You want it to be disabled by default on ffmpeg but enabled in
> ffplay?

Yeah that seems like it'd be most user friendly. Then 3rd party
developers can look at ffplay to see how to apply it in their own
players. mpv and vlc come to mind

> For the latter, something like:
> 
> > diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> > index 5212ad053e..217fc3e45a 100644
> > --- a/fftools/ffplay.c
> > +++ b/fftools/ffplay.c
> > @@ -36,6 +36,7 @@
> >  #include "libavutil/eval.h"
> >  #include "libavutil/mathematics.h"
> >  #include "libavutil/pixdesc.h"
> > +#include "libavutil/intreadwrite.h"
> >  #include "libavutil/imgutils.h"
> >  #include "libavutil/dict.h"
> >  #include "libavutil/fifo.h"
> > @@ -346,6 +347,7 @@ static const char **vfilters_list = NULL;
> >  static int nb_vfilters = 0;
> >  static char *afilters = NULL;
> >  static int autorotate = 1;
> > +static int apply_cropping = 1;
> >  static int find_stream_info = 1;
> >  static int filter_nbthreads = 0;
> > 
> > @@ -1922,6 +1924,27 @@ static int
> > configure_video_filters(AVFilterGraph *graph, VideoState *is, const
> > c
> >          }
> >      }
> > 
> > +    if (apply_cropping) {
> > +        size_t cropping_size;
> > +        uint8_t *cropping = av_stream_get_side_data(is->video_st,
> > AV_PKT_DATA_FRAME_CROPPING, &cropping_size);
> > +
> > +        if (cropping && cropping_size == sizeof(uint32_t) * 4) {
> > +            char crop_buf[64];
> > +            int top    = AV_RL32(cropping +  0);
> > +            int bottom = AV_RL32(cropping +  4);
> > +            int left   = AV_RL32(cropping +  8);
> > +            int right  = AV_RL32(cropping + 12);
> > +
> > +            if (top < 0 || bottom < 0 || left < 0 || right < 0)  {
> > +                ret = AVERROR(EINVAL);
> > +                goto fail;
> > +            }
> > +
> > +            snprintf(crop_buf, sizeof(crop_buf), "w=iw-%d-%d:h=ih-
> > %d-%d", left, right, top, bottom);
> > +            INSERT_FILT("crop", crop_buf);
> > +        }
> > +    }
> > +
> >      if ((ret = configure_filtergraph(graph, vfilters, filt_src,
> > last_filter)) < 0)
> >          goto fail;
> > 
> > @@ -3593,6 +3616,7 @@ static const OptionDef options[] = {
> >      { "scodec", HAS_ARG | OPT_STRING | OPT_EXPERT, {
> > &subtitle_codec_name }, "force subtitle decoder", "decoder_name" },
> >      { "vcodec", HAS_ARG | OPT_STRING | OPT_EXPERT, {   
> > &video_codec_name }, "force video decoder",    "decoder_name" },
> >      { "autorotate", OPT_BOOL, { &autorotate }, "automatically
> > rotate video", "" },
> > +    { "apply_cropping", OPT_BOOL, { &apply_cropping }, "apply
> > frame cropping", "" },
> >      { "find_stream_info", OPT_BOOL | OPT_INPUT | OPT_EXPERT, {
> > &find_stream_info },
> >          "read and decode the streams to fill missing information
> > with heuristics" },
> >      { "filter_threads", HAS_ARG | OPT_INT | OPT_EXPERT, {
> > &filter_nbthreads }, "number of filter threads per graph" },
> 
> Would do it, but i don't know if this affects the AVCodecContext
> option 
> of the same name too or not (to apply or not bitstream level
> cropping, 
> like the h264 one, which is obviously enabled by default).

Right, there's multiple levels of cropping

> To have it disabled on ffmpeg by default, i think the following would
> work (on top of this patch):
> 
> > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> > index 1209cf2046..a37c136cf9 100644
> > --- a/fftools/ffmpeg_demux.c
> > +++ b/fftools/ffmpeg_demux.c
> > @@ -1086,10 +1086,11 @@ static int ist_add(const OptionsContext *o,
> > Demuxer *d, AVStream *st)
> >      ist->autorotate = 1;
> >      MATCH_PER_STREAM_OPT(autorotate, i, ist->autorotate, ic, st);
> > 
> > -    ist->apply_cropping = 1;
> > +    ist->apply_cropping = -1;
> >      MATCH_PER_STREAM_OPT(apply_cropping, i, ist->apply_cropping,
> > ic, st);
> > 
> > -    av_dict_set_int(&o->g->codec_opts, "apply_cropping", ist-
> > >apply_cropping, 0);
> > +    if (ist->apply_cropping >= 0)
> > +        av_dict_set_int(&o->g->codec_opts, "apply_cropping", ist-
> > >apply_cropping, 0);
> > 
> >      MATCH_PER_STREAM_OPT(codec_tags, str, codec_tag, ic, st);
> >      if (codec_tag) {
> > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> > index 8cadb4732c..5df52ef718 100644
> > --- a/fftools/ffmpeg_filter.c
> > +++ b/fftools/ffmpeg_filter.c
> > @@ -1419,7 +1419,7 @@ static int
> > configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
> >              return ret;
> >      }
> > 
> > -    if (ist->apply_cropping && !(desc->flags &
> > AV_PIX_FMT_FLAG_HWACCEL)) {
> > +    if (ist->apply_cropping > 0 && !(desc->flags &
> > AV_PIX_FMT_FLAG_HWACCEL)) {
> >          size_t cropping_size;
> >          uint8_t *cropping = av_stream_get_side_data(ist->st,
> > AV_PKT_DATA_FRAME_CROPPING, &cropping_size);
> > 
> 
> And actually work fine with the AVCodecContext option.

What would be even neater is if cropping were applied automatically
when the container doesn't support such metadata. Possibly with a
message saying "container does not support display rectangle metadata.
applying cropping automatically. use -disable_cropping to disable". And
a corresponding -force_cropping. Or maybe that's too confusing? My
thinking here is that it should Just Work. If I convert an MXF file
with VBI data in the video essence, to AVI which does not have display
rectangle metadata, then I don't want to see the VBI crap in the output

/Tomas
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 5/5] fftools/ffmpeg: support applying container level cropping

[Anton Khirnov]

Quoting Tomas Härdin (2023-07-26)
> tis 2023-07-25 klockan 14:09 -0300 skrev James Almer:
> > Signed-off-by: James Almer <jamrial@gmail.com>
> > ---
> > Now inserting a filter into the graph.
> 
> This looks useful for MXF
> 
> > +    { "apply_cropping",   HAS_ARG | OPT_BOOL | OPT_SPEC |
> > +                          OPT_EXPERT |
> > OPT_INPUT,                                { .off =
> > OFFSET(apply_cropping) },
> > +        "Apply frame cropping instead of exporting it" },
> 
> Hm. Can this be applied automatically for ffplay? When transcoding I
> expect the typical use case is to not crop and to carry the metadata
> over.

Why? We do apply decoder cropping by default. There's also no guarantee
that your container will be able to write it, so it seems better to
apply it by default.

-- 
Anton Khirnov
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 5/5] fftools/ffmpeg: support applying container level cropping

[James Almer]

On 7/27/2023 8:13 AM, Anton Khirnov wrote:
> Quoting Tomas Härdin (2023-07-26)
>> tis 2023-07-25 klockan 14:09 -0300 skrev James Almer:
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>> Now inserting a filter into the graph.
>>
>> This looks useful for MXF
>>
>>> +    { "apply_cropping",   HAS_ARG | OPT_BOOL | OPT_SPEC |
>>> +                          OPT_EXPERT |
>>> OPT_INPUT,                                { .off =
>>> OFFSET(apply_cropping) },
>>> +        "Apply frame cropping instead of exporting it" },
>>
>> Hm. Can this be applied automatically for ffplay? When transcoding I
>> expect the typical use case is to not crop and to carry the metadata
>> over.
> 
> Why? We do apply decoder cropping by default. There's also no guarantee
> that your container will be able to write it, so it seems better to
> apply it by default.

I agree. In a transcoding scenario you want to apply the container level 
cropping since it's defining a subrectangle with the actual content 
meant for display, so why force the encoder handle pixels that were 
meant to be discarded to being with, potentially ruining encoding 
quality for neighboring pixels?

For codec copy scenarios though, the side data is going to be copied, so 
Tomas' idea of having muxers report they support writing it is good 
either way.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 5/5] fftools/ffmpeg: support applying container level cropping

[Tomas Härdin]

tor 2023-07-27 klockan 08:59 -0300 skrev James Almer:
> On 7/27/2023 8:13 AM, Anton Khirnov wrote:
> > Quoting Tomas Härdin (2023-07-26)
> > > tis 2023-07-25 klockan 14:09 -0300 skrev James Almer:
> > > > Signed-off-by: James Almer <jamrial@gmail.com>
> > > > ---
> > > > Now inserting a filter into the graph.
> > > 
> > > This looks useful for MXF
> > > 
> > > > +    { "apply_cropping",   HAS_ARG | OPT_BOOL | OPT_SPEC |
> > > > +                          OPT_EXPERT |
> > > > OPT_INPUT,                                { .off =
> > > > OFFSET(apply_cropping) },
> > > > +        "Apply frame cropping instead of exporting it" },
> > > 
> > > Hm. Can this be applied automatically for ffplay? When
> > > transcoding I
> > > expect the typical use case is to not crop and to carry the
> > > metadata
> > > over.
> > 
> > Why? We do apply decoder cropping by default. There's also no
> > guarantee
> > that your container will be able to write it, so it seems better to
> > apply it by default.

Not necessarily. Doing this by default may break some downstream
projects. The relevant metadata must be deleted if this is done, so
that the cropping isn't done twice when you get to playout.

> I agree. In a transcoding scenario you want to apply the container
> level 
> cropping since it's defining a subrectangle with the actual content 
> meant for display, so why force the encoder handle pixels that were 
> meant to be discarded to being with, potentially ruining encoding 
> quality for neighboring pixels?
> 
> For codec copy scenarios though, the side data is going to be copied,
> so 
> Tomas' idea of having muxers report they support writing it is good 
> either way.

My main concern is not losing pixels if we can avoid it, even if those
pixels are invisible. On the other hand, when transcoding, we could go
with always cropping unless the user requests otherwise. This has the
benefit of essence dimensions not changing with container. Also less
work for the encoder. But again, this is a behavior change that may
break things downstream.

Basically what I'm suggesting is that ffplay behave as playout. We
could have ffmpeg behave similarly but we should keep in mind this may
break some workflows.

/Tomas
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 5/5] fftools/ffmpeg: support applying container level cropping

[Cosmin Stejerean]

On Jul 27, 2023, at 4:13 AM, Anton Khirnov <anton@khirnov.net> wrote:

Quoting Tomas Härdin (2023-07-26)
tis 2023-07-25 klockan 14:09 -0300 skrev James Almer:
Signed-off-by: James Almer <jamrial@gmail.com>
---
Now inserting a filter into the graph.

This looks useful for MXF

+    { "apply_cropping",   HAS_ARG | OPT_BOOL | OPT_SPEC |
+                          OPT_EXPERT |
OPT_INPUT,                                { .off =
OFFSET(apply_cropping) },
+        "Apply frame cropping instead of exporting it" },

Hm. Can this be applied automatically for ffplay? When transcoding I
expect the typical use case is to not crop and to carry the metadata
over.

Why? We do apply decoder cropping by default. There's also no guarantee
that your container will be able to write it, so it seems better to
apply it by default.


I agree. I see this similar to rotation. And edit lists.

For remuxing we want to keep the metadata and have the muxer write it, but if we're going to transcode anyway we should simplify the stream (apply rotation, apply cropping, keep only visible frames, etc) and write out something as simple as possible. Anyone that doesn't want this can opt out of it like opting out of autorotation.

Not doing this means compatibility is worse when downstream players inevitably don't handle something properly (edit lists are still a mess in terms of compatibility for example). And of potentially displaying content that the user did not intend to be displayed.

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