Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: James Almer <jamrial@gmail.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH v2 5/5] fftools/ffmpeg: support applying container level cropping
Date: Wed, 26 Jul 2023 19:11:49 -0300
Message-ID: <9b8ebf02-70a9-4a9b-2951-474d4da821e2@gmail.com> (raw)
In-Reply-To: <5b466cb4640c2af26e6b8962b993fac3eaefc480.camel@haerdin.se>

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

  reply	other threads:[~2023-07-26 22:11 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-19 22:20 [FFmpeg-devel] [PATCH 1/5] avcodec/packet: add a decoded frame cropping side data type James Almer
2023-07-19 22:20 ` [FFmpeg-devel] [PATCH 2/5] avformat/dump: print Frame Cropping side data info James Almer
2023-07-19 22:20 ` [FFmpeg-devel] [PATCH 3/5] avformat/matroskadec: export cropping values James Almer
2023-07-19 22:20 ` [FFmpeg-devel] [PATCH 4/5] avformat/matroskaenc: support writing " James Almer
2023-07-19 22:20 ` [FFmpeg-devel] [PATCH 5/5] fftools/ffmpeg: support applying container level cropping James Almer
2023-07-20 19:08   ` Anton Khirnov
2023-07-20 19:25     ` James Almer
2023-07-20 19:31       ` Anton Khirnov
2023-07-20 19:39         ` James Almer
2023-07-25 17:09 ` [FFmpeg-devel] [PATCH v2 " James Almer
2023-07-26 21:42   ` Tomas Härdin
2023-07-26 22:11     ` James Almer [this message]
2023-07-27 11:07       ` Tomas Härdin
2023-07-27 11:13     ` Anton Khirnov
2023-07-27 11:59       ` James Almer
2023-07-31 15:53         ` Tomas Härdin
     [not found]       ` <3EF017F1-A8DB-4934-A86C-8E17BC067DA0@cosmin.at>
2023-08-01 18:51         ` Cosmin Stejerean

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9b8ebf02-70a9-4a9b-2951-474d4da821e2@gmail.com \
    --to=jamrial@gmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git