[FFmpeg-devel] [PATCH 0/7] Container level frame cropping parameters

[FFmpeg-devel] [PATCH 0/7] Container level frame cropping parameters

[James Almer]

This is a simple set to add support to stream wide, container level cropping
parameters, as featured in formats like Matroska and ISOBMFF. It's needed for
things like AV1 streams generated by certain hardware encoders that produce
dimension aligned output, and unlike H26x, can't export cropping info within
the bitstream.
In this set i add the packet side data type, mux and demux support to Matroska,
and handling code to ffmpeg and ffplay. Missing is support for clap boxes in
ISOBMFF, and exporting the relevant side data from encoders like AMF.

It's a rebased and updated version to the set i sent a month ago.

James Almer (7):
  avcodec/packet: add a decoded frame cropping side data type
  avformat/dump: print Frame Cropping side data info
  avformat/matroskadec: export cropping values
  avformat/avformat: add a flag to signal muxers that support storing
    cropping values
  avformat/matroskaenc: support writing cropping values
  fftools/ffmpeg: support applying container level cropping
  fftools/ffplay: support applying container level cropping

 fftools/ffmpeg.h          |  3 ++
 fftools/ffmpeg_demux.c    |  6 ++++
 fftools/ffmpeg_enc.c      | 24 ++++++++--------
 fftools/ffmpeg_filter.c   | 23 +++++++++++++++
 fftools/ffmpeg_opt.c      |  3 ++
 fftools/ffplay.c          | 26 +++++++++++++++++
 libavcodec/packet.h       | 14 ++++++++++
 libavformat/avformat.h    |  1 +
 libavformat/dump.c        | 21 ++++++++++++++
 libavformat/matroskadec.c | 53 ++++++++++++++++++++++++++++-------
 libavformat/matroskaenc.c | 59 ++++++++++++++++++++++++++++++---------
 libavformat/mux.c         |  8 ++++++
 12 files changed, 207 insertions(+), 34 deletions(-)

-- 
2.42.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 1/7] 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 b19409b719..6053d43c44 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.42.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 1/7] avcodec/packet: add a decoded frame cropping side data type

[Andreas Rheinhardt]

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 b19409b719..6053d43c44 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

The data in ISOBMFF clap boxes is more general than that, therefore this
should be used so that this data can be preserved during remuxes.
See the earlier patchset by Neil Birkbeck for it:
https://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/261465.html (there
is more of this discussion in subsequent months).

- Andreas

_______________________________________________
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/7] 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 c0868a1bb3..8986e7c32a 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.42.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 2/7] avformat/dump: print Frame Cropping side data info

[Anton Khirnov]

Quoting James Almer (2023-10-07 18:24:58)
> 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 c0868a1bb3..8986e7c32a 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);

%PRIu32?

Also, some user indication of which is which would be nice, like l:5/r:1/...

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

[FFmpeg-devel] [PATCH 3/7] avformat/matroskadec: export cropping values

[James Almer]

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

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index c8acf91970..0e98e97957 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 } },
@@ -2953,14 +2959,30 @@ static int mkv_parse_video(MatroskaTrack *track, AVStream *st,
 
     if (track->video.display_unit < MATROSKA_VIDEO_DISPLAYUNIT_UNKNOWN) {
         if (track->video.display_width && track->video.display_height &&
-            par->height  < INT64_MAX / track->video.display_width  / display_width_mul &&
-            par->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,
-                      par->height * track->video.display_width  * display_width_mul,
-                      par->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;
+        AVPacketSideData *sd = av_packet_side_data_new(&st->codecpar->coded_side_data,
+                                                       &st->codecpar->nb_coded_side_data,
+                                                       AV_PKT_DATA_FRAME_CROPPING,
+                                                       sizeof(uint32_t) * 4, 0);
+        if (!sd)
+            return AVERROR(ENOMEM);
+
+        cropping = sd->data;
+        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 (par->codec_id != AV_CODEC_ID_HEVC)
         sti->need_parsing = AVSTREAM_PARSE_HEADERS;
 
@@ -3126,10 +3148,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;
         } else if (track->type == MATROSKA_TRACK_TYPE_AUDIO) {
             if (!track->audio.out_samplerate)
                 track->audio.out_samplerate = track->audio.samplerate;
-- 
2.42.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/7] avformat/avformat: add a flag to signal muxers that support storing cropping values

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/avformat.h | 1 +
 libavformat/mux.c      | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 9e7eca007e..c099ca8a01 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -500,6 +500,7 @@ typedef struct AVProbeData {
                                         The user or muxer can override this through
                                         AVFormatContext.avoid_negative_ts
                                         */
+#define AVFMT_CROPPING     0x80000 /**< Format supports storing cropping values */
 
 #define AVFMT_SEEK_TO_PTS   0x4000000 /**< Seeking is based on PTS */
 
diff --git a/libavformat/mux.c b/libavformat/mux.c
index c7877c5d98..c5e8a4ca74 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -188,6 +188,7 @@ static int init_muxer(AVFormatContext *s, AVDictionary **options)
     AVDictionary *tmp = NULL;
     const FFOutputFormat *of = ffofmt(s->oformat);
     AVDictionaryEntry *e;
+    int warned_crop = 0;
     int ret = 0;
 
     if (options)
@@ -276,6 +277,13 @@ FF_ENABLE_DEPRECATION_WARNINGS
                     goto fail;
                 }
             }
+            if (!warned_crop && !(s->oformat->flags & AVFMT_CROPPING) &&
+                av_packet_side_data_get(st->codecpar->coded_side_data,
+                                        st->codecpar->nb_coded_side_data,
+                                        AV_PKT_DATA_FRAME_CROPPING)) {
+                av_log(s, AV_LOG_WARNING, "Muxer does not support storing cropping values\n");
+                warned_crop = 1;
+            }
             break;
         }
 
-- 
2.42.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 4/7] avformat/avformat: add a flag to signal muxers that support storing cropping values

[Anton Khirnov]

Quoting James Almer (2023-10-07 18:25:00)
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavformat/avformat.h | 1 +
>  libavformat/mux.c      | 8 ++++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 9e7eca007e..c099ca8a01 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -500,6 +500,7 @@ typedef struct AVProbeData {
>                                          The user or muxer can override this through
>                                          AVFormatContext.avoid_negative_ts
>                                          */
> +#define AVFMT_CROPPING     0x80000 /**< Format supports storing cropping values */

I have mixed feeelings about this patch, for a bunch of reasons:
* It is quite ad-hoc - we don't do this for other side data types, and
  this approach would not scale if we did.
* If we do want to signal this, we probably want to distinguish between
  support for global and per-packet values.
* How do you expect this to be useful to the callers? I don't see this
  flag actually being used in the ffmpeg CLI patch.

-- 
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 4/7] avformat/avformat: add a flag to signal muxers that support storing cropping values

[James Almer]

On 10/10/2023 8:09 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-10-07 18:25:00)
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavformat/avformat.h | 1 +
>>   libavformat/mux.c      | 8 ++++++++
>>   2 files changed, 9 insertions(+)
>>
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index 9e7eca007e..c099ca8a01 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -500,6 +500,7 @@ typedef struct AVProbeData {
>>                                           The user or muxer can override this through
>>                                           AVFormatContext.avoid_negative_ts
>>                                           */
>> +#define AVFMT_CROPPING     0x80000 /**< Format supports storing cropping values */
> 
> I have mixed feeelings about this patch, for a bunch of reasons:
> * It is quite ad-hoc - we don't do this for other side data types, and
>    this approach would not scale if we did.
> * If we do want to signal this, we probably want to distinguish between
>    support for global and per-packet values.

This patch came to be after some discussion from the first iteration of 
the set, where concerns about the cropping information being silently 
lost if apply_cropping was disabled during a transcoding or codec copy 
scenario where the output format didn't support storing said values.

> * How do you expect this to be useful to the callers? I don't see this
>    flag actually being used in the ffmpeg CLI patch.

It's a format flag. Muxers use it, and the generic mux.c code will print 
a warning if needed.
_______________________________________________
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 4/7] avformat/avformat: add a flag to signal muxers that support storing cropping values

[Anton Khirnov]

Quoting James Almer (2023-10-10 13:13:46)
> On 10/10/2023 8:09 AM, Anton Khirnov wrote:
> > Quoting James Almer (2023-10-07 18:25:00)
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >>   libavformat/avformat.h | 1 +
> >>   libavformat/mux.c      | 8 ++++++++
> >>   2 files changed, 9 insertions(+)
> >>
> >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >> index 9e7eca007e..c099ca8a01 100644
> >> --- a/libavformat/avformat.h
> >> +++ b/libavformat/avformat.h
> >> @@ -500,6 +500,7 @@ typedef struct AVProbeData {
> >>                                           The user or muxer can override this through
> >>                                           AVFormatContext.avoid_negative_ts
> >>                                           */
> >> +#define AVFMT_CROPPING     0x80000 /**< Format supports storing cropping values */
> > 
> > I have mixed feeelings about this patch, for a bunch of reasons:
> > * It is quite ad-hoc - we don't do this for other side data types, and
> >    this approach would not scale if we did.
> > * If we do want to signal this, we probably want to distinguish between
> >    support for global and per-packet values.
> 
> This patch came to be after some discussion from the first iteration of 
> the set, where concerns about the cropping information being silently 
> lost if apply_cropping was disabled during a transcoding or codec copy 
> scenario where the output format didn't support storing said values.
> 
> > * How do you expect this to be useful to the callers? I don't see this
> >    flag actually being used in the ffmpeg CLI patch.
> 
> It's a format flag. Muxers use it, and the generic mux.c code will print 
> a warning if needed.

Why is it public then?

-- 
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 4/7] avformat/avformat: add a flag to signal muxers that support storing cropping values

[James Almer]

On 10/10/2023 8:16 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-10-10 13:13:46)
>> On 10/10/2023 8:09 AM, Anton Khirnov wrote:
>>> Quoting James Almer (2023-10-07 18:25:00)
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>>    libavformat/avformat.h | 1 +
>>>>    libavformat/mux.c      | 8 ++++++++
>>>>    2 files changed, 9 insertions(+)
>>>>
>>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>>> index 9e7eca007e..c099ca8a01 100644
>>>> --- a/libavformat/avformat.h
>>>> +++ b/libavformat/avformat.h
>>>> @@ -500,6 +500,7 @@ typedef struct AVProbeData {
>>>>                                            The user or muxer can override this through
>>>>                                            AVFormatContext.avoid_negative_ts
>>>>                                            */
>>>> +#define AVFMT_CROPPING     0x80000 /**< Format supports storing cropping values */
>>>
>>> I have mixed feeelings about this patch, for a bunch of reasons:
>>> * It is quite ad-hoc - we don't do this for other side data types, and
>>>     this approach would not scale if we did.
>>> * If we do want to signal this, we probably want to distinguish between
>>>     support for global and per-packet values.
>>
>> This patch came to be after some discussion from the first iteration of
>> the set, where concerns about the cropping information being silently
>> lost if apply_cropping was disabled during a transcoding or codec copy
>> scenario where the output format didn't support storing said values.
>>
>>> * How do you expect this to be useful to the callers? I don't see this
>>>     flag actually being used in the ffmpeg CLI patch.
>>
>> It's a format flag. Muxers use it, and the generic mux.c code will print
>> a warning if needed.
> 
> Why is it public then?

So the library user can know beforehand if the cropping information will 
be lost or not, and choose accordingly. The warning is there for the 
cases where it was ignored.

I can add a check to the CLI for it, but other than to abort or outright 
ignore the user request to not apply cropping i don't see what it could 
do, as mux.c already prints a warning.
_______________________________________________
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 4/7] avformat/avformat: add a flag to signal muxers that support storing cropping values

[Anton Khirnov]

Quoting James Almer (2023-10-10 13:21:42)
> On 10/10/2023 8:16 AM, Anton Khirnov wrote:
> > Quoting James Almer (2023-10-10 13:13:46)
> >> On 10/10/2023 8:09 AM, Anton Khirnov wrote:
> >>> Quoting James Almer (2023-10-07 18:25:00)
> >>>> Signed-off-by: James Almer <jamrial@gmail.com>
> >>>> ---
> >>>>    libavformat/avformat.h | 1 +
> >>>>    libavformat/mux.c      | 8 ++++++++
> >>>>    2 files changed, 9 insertions(+)
> >>>>
> >>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >>>> index 9e7eca007e..c099ca8a01 100644
> >>>> --- a/libavformat/avformat.h
> >>>> +++ b/libavformat/avformat.h
> >>>> @@ -500,6 +500,7 @@ typedef struct AVProbeData {
> >>>>                                            The user or muxer can override this through
> >>>>                                            AVFormatContext.avoid_negative_ts
> >>>>                                            */
> >>>> +#define AVFMT_CROPPING     0x80000 /**< Format supports storing cropping values */
> >>>
> >>> I have mixed feeelings about this patch, for a bunch of reasons:
> >>> * It is quite ad-hoc - we don't do this for other side data types, and
> >>>     this approach would not scale if we did.
> >>> * If we do want to signal this, we probably want to distinguish between
> >>>     support for global and per-packet values.
> >>
> >> This patch came to be after some discussion from the first iteration of
> >> the set, where concerns about the cropping information being silently
> >> lost if apply_cropping was disabled during a transcoding or codec copy
> >> scenario where the output format didn't support storing said values.
> >>
> >>> * How do you expect this to be useful to the callers? I don't see this
> >>>     flag actually being used in the ffmpeg CLI patch.
> >>
> >> It's a format flag. Muxers use it, and the generic mux.c code will print
> >> a warning if needed.
> > 
> > Why is it public then?
> 
> So the library user can know beforehand if the cropping information will 
> be lost or not, and choose accordingly. The warning is there for the 
> cases where it was ignored.
> 
> I can add a check to the CLI for it, but other than to abort or outright 
> ignore the user request to not apply cropping i don't see what it could 
> do, as mux.c already prints a warning.

As I said above - there are MANY similar bits of information that the
muxer may or may not write. You cannot handle all of them with this
approach, and singling out cropping seems horribly ad-hoc to me.

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

[FFmpeg-devel] [PATCH 5/7] avformat/matroskaenc: support writing cropping values

[James Almer]

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

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 1457a6890c..9005237a83 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1751,8 +1751,10 @@ 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];
+    const AVPacketSideData *sd;
     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;
     int ret;
 
     ebml_writer_open_master(&writer, MATROSKA_ID_TRACKVIDEO);
@@ -1775,25 +1777,58 @@ static int mkv_write_track_video(AVFormatContext *s, MatroskaMuxContext *mkv,
          (tag = av_dict_get( s->metadata, "alpha_mode", NULL, 0))) && strtol(tag->value, NULL, 0))
         ebml_writer_add_uint(&writer, MATROSKA_ID_VIDEOALPHAMODE, 1);
 
+    sd = av_packet_side_data_get(st->codecpar->coded_side_data,
+                                 st->codecpar->nb_coded_side_data,
+                                 AV_PKT_DATA_FRAME_CROPPING);
+    if (sd) {
+        const uint8_t *cropping = sd->data;
+        size_t cropped_size = sd->size;
+        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);
@@ -1805,9 +1840,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);
@@ -3555,10 +3590,9 @@ const FFOutputFormat ff_matroska_muxer = {
     .write_trailer     = mkv_write_trailer,
     .p.flags           = AVFMT_GLOBALHEADER | AVFMT_VARIABLE_FPS |
 #if FF_API_ALLOW_FLUSH
-                         AVFMT_TS_NONSTRICT | AVFMT_ALLOW_FLUSH,
-#else
-                         AVFMT_TS_NONSTRICT,
+                         AVFMT_ALLOW_FLUSH |
 #endif
+                         AVFMT_TS_NONSTRICT | AVFMT_CROPPING,
     .p.codec_tag       = (const AVCodecTag* const []){
          ff_codec_bmp_tags, ff_codec_wav_tags,
          additional_audio_tags, additional_subtitle_tags, 0
@@ -3599,10 +3633,9 @@ const FFOutputFormat ff_webm_muxer = {
     .check_bitstream   = mkv_check_bitstream,
     .p.flags           = AVFMT_GLOBALHEADER | AVFMT_VARIABLE_FPS |
 #if FF_API_ALLOW_FLUSH
-                         AVFMT_TS_NONSTRICT | AVFMT_ALLOW_FLUSH,
-#else
-                         AVFMT_TS_NONSTRICT,
+                         AVFMT_ALLOW_FLUSH |
 #endif
+                         AVFMT_TS_NONSTRICT | AVFMT_CROPPING,
     .p.priv_class      = &matroska_webm_class,
     .flags_internal    = FF_FMT_ALLOW_FLUSH,
 };
-- 
2.42.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 6/7] fftools/ffmpeg: support applying container level cropping

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
 fftools/ffmpeg.h        |  3 +++
 fftools/ffmpeg_demux.c  |  6 ++++++
 fftools/ffmpeg_enc.c    | 24 +++++++++++++-----------
 fftools/ffmpeg_filter.c | 23 +++++++++++++++++++++++
 fftools/ffmpeg_opt.c    |  3 +++
 5 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index b059ecbb9f..94ada9d586 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -155,6 +155,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;
@@ -352,6 +354,7 @@ typedef struct InputStream {
 #endif
 
     int autorotate;
+    int apply_cropping;
 
     int fix_sub_duration;
 
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index c71edf01a5..53fe584959 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 be35465c28..27dd28172c 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -467,17 +467,19 @@ int enc_open(OutputStream *ost, AVFrame *frame)
     if (ist) {
         int i;
         for (i = 0; i < ist->st->codecpar->nb_coded_side_data; i++) {
-            AVPacketSideData *sd_src = &ist->st->codecpar->coded_side_data[i];
-            if (sd_src->type != AV_PKT_DATA_CPB_PROPERTIES) {
-                AVPacketSideData *sd_dst = av_packet_side_data_new(&ost->par_in->coded_side_data,
-                                                                   &ost->par_in->nb_coded_side_data,
-                                                                   sd_src->type, sd_src->size, 0);
-                if (!sd_dst)
-                    return AVERROR(ENOMEM);
-                memcpy(sd_dst->data, sd_src->data, sd_src->size);
-                if (ist->autorotate && sd_src->type == AV_PKT_DATA_DISPLAYMATRIX)
-                    av_display_rotation_set((int32_t *)sd_dst->data, 0);
-            }
+            AVPacketSideData *sd_dst, *sd_src = &ist->st->codecpar->coded_side_data[i];
+            if (sd_src->type == AV_PKT_DATA_CPB_PROPERTIES)
+                continue;
+            if (ist->apply_cropping && sd_src->type == AV_PKT_DATA_FRAME_CROPPING)
+                continue;
+            sd_dst = av_packet_side_data_new(&ost->par_in->coded_side_data,
+                                             &ost->par_in->nb_coded_side_data,
+                                             sd_src->type, sd_src->size, 0);
+            if (!sd_dst)
+                return AVERROR(ENOMEM);
+            memcpy(sd_dst->data, sd_src->data, sd_src->size);
+            if (ist->autorotate && sd_src->type == AV_PKT_DATA_DISPLAYMATRIX)
+                av_display_rotation_set((int32_t *)sd_dst->data, 0);
         }
     }
 
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index b71a84185a..d17b9fc45a 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"
@@ -1381,6 +1382,28 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
     desc = av_pix_fmt_desc_get(ifp->format);
     av_assert0(desc);
 
+    if (ist->apply_cropping) {
+        AVPacketSideData *sd = av_packet_side_data_get(ist->st->codecpar->coded_side_data,
+                                                       ist->st->codecpar->nb_coded_side_data,
+                                                       AV_PKT_DATA_FRAME_CROPPING);
+
+        if (sd->data && sd->size == sizeof(uint32_t) * 4) {
+            char crop_buf[64];
+            int top    = AV_RL32(sd->data +  0);
+            int bottom = AV_RL32(sd->data +  4);
+            int left   = AV_RL32(sd->data +  8);
+            int right  = AV_RL32(sd->data + 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;
+        }
+    }
+
     // TODO: insert hwaccel enabled filters like transpose_vaapi into the graph
     if (ist->autorotate && !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
         const AVPacketSideData *sd = NULL;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 304471dd03..1ef131aa3a 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1737,6 +1737,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.42.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 6/7] fftools/ffmpeg: support applying container level cropping

[Anton Khirnov]

Quoting James Almer (2023-10-07 18:25:02)
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  fftools/ffmpeg.h        |  3 +++
>  fftools/ffmpeg_demux.c  |  6 ++++++
>  fftools/ffmpeg_enc.c    | 24 +++++++++++++-----------
>  fftools/ffmpeg_filter.c | 23 +++++++++++++++++++++++
>  fftools/ffmpeg_opt.c    |  3 +++
>  5 files changed, 48 insertions(+), 11 deletions(-)
> 
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index b059ecbb9f..94ada9d586 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -155,6 +155,8 @@ typedef struct OptionsContext {
>      int        nb_hwaccel_output_formats;
>      SpecifierOpt *autorotate;
>      int        nb_autorotate;
> +    SpecifierOpt *apply_cropping;
> +    int        nb_apply_cropping;

I would prefer to handle this similarly to the "display_rotation"
option - give the user the option to override container-stored values
(which includes setting them to zero, to ignore them).

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

[FFmpeg-devel] [PATCH 7/7] fftools/ffplay: support applying container level cropping

[James Almer]

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

diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index d8c69e10bc..5c213bf0d8 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"
@@ -348,6 +349,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;
 
@@ -1910,6 +1912,28 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
     last_filter = filt_ctx;                                                  \
 } while (0)
 
+    if (apply_cropping) {
+        AVPacketSideData *sd = av_packet_side_data_get(is->video_st->codecpar->coded_side_data,
+                                                       is->video_st->codecpar->nb_coded_side_data,
+                                                       AV_PKT_DATA_FRAME_CROPPING);
+
+        if (sd->data && sd->size == sizeof(uint32_t) * 4) {
+            char crop_buf[64];
+            int top    = AV_RL32(sd->data +  0);
+            int bottom = AV_RL32(sd->data +  4);
+            int left   = AV_RL32(sd->data +  8);
+            int right  = AV_RL32(sd->data + 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 (autorotate) {
         double theta = 0.0;
         int32_t *displaymatrix = NULL;
@@ -2610,6 +2634,7 @@ static int stream_component_open(VideoState *is, int stream_index)
         av_dict_set(&opts, "threads", "auto", 0);
     if (stream_lowres)
         av_dict_set_int(&opts, "lowres", stream_lowres, 0);
+    av_dict_set_int(&opts, "apply_cropping", apply_cropping, 0);
 
     av_dict_set(&opts, "flags", "+copy_opaque", AV_DICT_MULTIKEY);
 
@@ -3608,6 +3633,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" },
-- 
2.42.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 7/7] fftools/ffplay: support applying container level cropping

[Dmitrii Ovchinnikov]

I tested these patches with the AMF AV1 encoder(by adding
AV_PKT_DATA_FRAME_CROPPING)
It works fine.

I suggest a small change to the code for better safety. Change this part:
>>if (sd->data && sd->size == sizeof(uint32_t) * 4) {
to:
if (sd && sd->data && sd->size == sizeof(uint32_t) * 4) {

This change helps avoid crashes when the cropping data isn't filled in.
_______________________________________________
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 0/7] Container level frame cropping parameters

[Anton Khirnov]

Quoting James Almer (2023-10-07 18:24:56)
> This is a simple set to add support to stream wide, container level cropping
> parameters, as featured in formats like Matroska and ISOBMFF. It's needed for
> things like AV1 streams generated by certain hardware encoders that produce
> dimension aligned output, and unlike H26x, can't export cropping info within
> the bitstream.
> In this set i add the packet side data type, mux and demux support to Matroska,
> and handling code to ffmpeg and ffplay. Missing is support for clap boxes in
> ISOBMFF, and exporting the relevant side data from encoders like AMF.
> 
> It's a rebased and updated version to the set i sent a month ago.

Why is there no lavc-decoder handling? I would expect cropping side data
submitted to lavc to be somehow propagated to decoded frames.
Though there is a question of how to combine side data with the
codec-level cropping.

-- 
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 0/7] Container level frame cropping parameters

[James Almer]

On 10/10/2023 8:17 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-10-07 18:24:56)
>> This is a simple set to add support to stream wide, container level cropping
>> parameters, as featured in formats like Matroska and ISOBMFF. It's needed for
>> things like AV1 streams generated by certain hardware encoders that produce
>> dimension aligned output, and unlike H26x, can't export cropping info within
>> the bitstream.
>> In this set i add the packet side data type, mux and demux support to Matroska,
>> and handling code to ffmpeg and ffplay. Missing is support for clap boxes in
>> ISOBMFF, and exporting the relevant side data from encoders like AMF.
>>
>> It's a rebased and updated version to the set i sent a month ago.
> 
> Why is there no lavc-decoder handling? I would expect cropping side data
> submitted to lavc to be somehow propagated to decoded frames.
> Though there is a question of how to combine side data with the
> codec-level cropping.

I thought about adding lavc support alongside the CLI handling, yeah. 
And it should be a matter of adding the values to the existing ones in 
frame->crop_* as exported by decoders, which av_frame_apply_cropping() 
will then handle.

And the reason i add support to the CLI by including a filter instead of 
leaving it up to lavc is that av_frame_apply_cropping() may not do exact 
cropping if AV_CODEC_FLAG_UNALIGNED is not set.
_______________________________________________
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 0/7] Container level frame cropping parameters

[Anton Khirnov]

Quoting James Almer (2023-10-10 13:44:19)
> On 10/10/2023 8:17 AM, Anton Khirnov wrote:
> > Quoting James Almer (2023-10-07 18:24:56)
> >> This is a simple set to add support to stream wide, container level cropping
> >> parameters, as featured in formats like Matroska and ISOBMFF. It's needed for
> >> things like AV1 streams generated by certain hardware encoders that produce
> >> dimension aligned output, and unlike H26x, can't export cropping info within
> >> the bitstream.
> >> In this set i add the packet side data type, mux and demux support to Matroska,
> >> and handling code to ffmpeg and ffplay. Missing is support for clap boxes in
> >> ISOBMFF, and exporting the relevant side data from encoders like AMF.
> >>
> >> It's a rebased and updated version to the set i sent a month ago.
> > 
> > Why is there no lavc-decoder handling? I would expect cropping side data
> > submitted to lavc to be somehow propagated to decoded frames.
> > Though there is a question of how to combine side data with the
> > codec-level cropping.
> 
> I thought about adding lavc support alongside the CLI handling, yeah. 
> And it should be a matter of adding the values to the existing ones in 
> frame->crop_* as exported by decoders, which av_frame_apply_cropping() 
> will then handle.
> 
> And the reason i add support to the CLI by including a filter instead of 
> leaving it up to lavc is that av_frame_apply_cropping() may not do exact 
> cropping if AV_CODEC_FLAG_UNALIGNED is not set.

I intend to have the CLI always set AVCodecContext.apply_cropping=0 and
have it handled by lavfi for this exact reason, just didn't get to
actually doing it yet. Feel free and welcome to do it yourself.

-- 
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 0/7] Container level frame cropping parameters

[James Almer]

On 10/10/2023 8:54 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-10-10 13:44:19)
>> On 10/10/2023 8:17 AM, Anton Khirnov wrote:
>>> Quoting James Almer (2023-10-07 18:24:56)
>>>> This is a simple set to add support to stream wide, container level cropping
>>>> parameters, as featured in formats like Matroska and ISOBMFF. It's needed for
>>>> things like AV1 streams generated by certain hardware encoders that produce
>>>> dimension aligned output, and unlike H26x, can't export cropping info within
>>>> the bitstream.
>>>> In this set i add the packet side data type, mux and demux support to Matroska,
>>>> and handling code to ffmpeg and ffplay. Missing is support for clap boxes in
>>>> ISOBMFF, and exporting the relevant side data from encoders like AMF.
>>>>
>>>> It's a rebased and updated version to the set i sent a month ago.
>>>
>>> Why is there no lavc-decoder handling? I would expect cropping side data
>>> submitted to lavc to be somehow propagated to decoded frames.
>>> Though there is a question of how to combine side data with the
>>> codec-level cropping.
>>
>> I thought about adding lavc support alongside the CLI handling, yeah.
>> And it should be a matter of adding the values to the existing ones in
>> frame->crop_* as exported by decoders, which av_frame_apply_cropping()
>> will then handle.
>>
>> And the reason i add support to the CLI by including a filter instead of
>> leaving it up to lavc is that av_frame_apply_cropping() may not do exact
>> cropping if AV_CODEC_FLAG_UNALIGNED is not set.
> 
> I intend to have the CLI always set AVCodecContext.apply_cropping=0 and
> have it handled by lavfi for this exact reason, just didn't get to
> actually doing it yet. Feel free and welcome to do it yourself.

The cropping filter doesn't look at the frame cropping fields, though. 
It in fact exports cropping into them if you pass it hw frames. It only 
looks at input arguments. So it doesn't seem like that can be done 
without changing the filter.
_______________________________________________
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 0/7] Container level frame cropping parameters

[Anton Khirnov]

Quoting James Almer (2023-10-10 13:58:00)
> On 10/10/2023 8:54 AM, Anton Khirnov wrote:
> > Quoting James Almer (2023-10-10 13:44:19)
> >> On 10/10/2023 8:17 AM, Anton Khirnov wrote:
> >>> Quoting James Almer (2023-10-07 18:24:56)
> >>>> This is a simple set to add support to stream wide, container level cropping
> >>>> parameters, as featured in formats like Matroska and ISOBMFF. It's needed for
> >>>> things like AV1 streams generated by certain hardware encoders that produce
> >>>> dimension aligned output, and unlike H26x, can't export cropping info within
> >>>> the bitstream.
> >>>> In this set i add the packet side data type, mux and demux support to Matroska,
> >>>> and handling code to ffmpeg and ffplay. Missing is support for clap boxes in
> >>>> ISOBMFF, and exporting the relevant side data from encoders like AMF.
> >>>>
> >>>> It's a rebased and updated version to the set i sent a month ago.
> >>>
> >>> Why is there no lavc-decoder handling? I would expect cropping side data
> >>> submitted to lavc to be somehow propagated to decoded frames.
> >>> Though there is a question of how to combine side data with the
> >>> codec-level cropping.
> >>
> >> I thought about adding lavc support alongside the CLI handling, yeah.
> >> And it should be a matter of adding the values to the existing ones in
> >> frame->crop_* as exported by decoders, which av_frame_apply_cropping()
> >> will then handle.
> >>
> >> And the reason i add support to the CLI by including a filter instead of
> >> leaving it up to lavc is that av_frame_apply_cropping() may not do exact
> >> cropping if AV_CODEC_FLAG_UNALIGNED is not set.
> > 
> > I intend to have the CLI always set AVCodecContext.apply_cropping=0 and
> > have it handled by lavfi for this exact reason, just didn't get to
> > actually doing it yet. Feel free and welcome to do it yourself.
> 
> The cropping filter doesn't look at the frame cropping fields, though. 
> It in fact exports cropping into them if you pass it hw frames. It only 
> looks at input arguments. So it doesn't seem like that can be done 
> without changing the filter.

I mean ffmpeg CLI would look at the frame cropping fields and insert the
crop filter accordingly, as it does for other bits of metadata.

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