diff mbox series

[FFmpeg-devel,5/6] fftools/ffmpeg: support applying container level cropping

Message ID 20240529214632.9843-5-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/6] avcodec/packet: add a decoded frame cropping side data type | expand

Checks

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

Commit Message

James Almer May 29, 2024, 9:46 p.m. UTC
Signed-off-by: James Almer <jamrial@gmail.com>
---
 fftools/ffmpeg.h        |  7 +++++++
 fftools/ffmpeg_demux.c  | 16 ++++++++++++++++
 fftools/ffmpeg_filter.c | 11 +++++++++++
 fftools/ffmpeg_opt.c    |  3 +++
 4 files changed, 37 insertions(+)

Comments

Lynne May 30, 2024, 1:01 a.m. UTC | #1
On 29/05/2024 23:46, James Almer wrote:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>   fftools/ffmpeg.h        |  7 +++++++
>   fftools/ffmpeg_demux.c  | 16 ++++++++++++++++
>   fftools/ffmpeg_filter.c | 11 +++++++++++
>   fftools/ffmpeg_opt.c    |  3 +++
>   4 files changed, 37 insertions(+)
> 
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index fe75706afd..f908e16549 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -155,6 +155,7 @@ typedef struct OptionsContext {
>       SpecifierOptList hwaccel_devices;
>       SpecifierOptList hwaccel_output_formats;
>       SpecifierOptList autorotate;
> +    SpecifierOptList apply_cropping;
>   
>       /* output options */
>       StreamMap *stream_maps;
> @@ -239,6 +240,7 @@ enum IFilterFlags {
>       IFILTER_FLAG_AUTOROTATE     = (1 << 0),
>       IFILTER_FLAG_REINIT         = (1 << 1),
>       IFILTER_FLAG_CFR            = (1 << 2),
> +    IFILTER_FLAG_CROP           = (1 << 3),
>   };
>   
>   typedef struct InputFilterOptions {
> @@ -254,6 +256,11 @@ typedef struct InputFilterOptions {
>        * accurate */
>       AVRational          framerate;
>   
> +    unsigned crop_top;
> +    unsigned crop_bottom;
> +    unsigned crop_left;
> +    unsigned crop_right;
> +
>       int                 sub2video_width;
>       int                 sub2video_height;
>   
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index 1ca8d804ae..4178b8f840 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -66,6 +66,7 @@ typedef struct DemuxStream {
>       int                      have_sub2video;
>       int                      reinit_filters;
>       int                      autorotate;
> +    int                      apply_cropping;
>   
>   
>       int                      wrap_correction_done;
> @@ -1000,11 +1001,20 @@ int ist_filter_add(InputStream *ist, InputFilter *ifilter, int is_simple,
>       ist->filters[ist->nb_filters - 1] = ifilter;
>   
>       if (ist->par->codec_type == AVMEDIA_TYPE_VIDEO) {
> +        const 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 (ist->framerate.num > 0 && ist->framerate.den > 0) {
>               opts->framerate = ist->framerate;
>               opts->flags |= IFILTER_FLAG_CFR;
>           } else
>               opts->framerate = av_guess_frame_rate(d->f.ctx, ist->st, NULL);
> +        if (sd && sd->size == sizeof(uint32_t) * 4) {
> +            opts->crop_top    = AV_RL32(sd->data +  0);
> +            opts->crop_bottom = AV_RL32(sd->data +  4);
> +            opts->crop_left   = AV_RL32(sd->data +  8);
> +            opts->crop_right  = AV_RL32(sd->data + 12);
> +        }
>       } else if (ist->par->codec_type == AVMEDIA_TYPE_SUBTITLE) {
>           /* Compute the size of the canvas for the subtitles stream.
>              If the subtitles codecpar has set a size, use it. Otherwise use the
> @@ -1059,6 +1069,7 @@ int ist_filter_add(InputStream *ist, InputFilter *ifilter, int is_simple,
>           return AVERROR(ENOMEM);
>   
>       opts->flags |= IFILTER_FLAG_AUTOROTATE * !!(ds->autorotate) |
> +                   IFILTER_FLAG_CROP       * !!(ds->apply_cropping) |
>                      IFILTER_FLAG_REINIT     * !!(ds->reinit_filters);
>   
>       return ds->sch_idx_dec;
> @@ -1241,6 +1252,9 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>       ds->autorotate = 1;
>       MATCH_PER_STREAM_OPT(autorotate, i, ds->autorotate, ic, st);
>   
> +    ds->apply_cropping = 1;
> +    MATCH_PER_STREAM_OPT(apply_cropping, i, ds->apply_cropping, ic, st);
> +
>       MATCH_PER_STREAM_OPT(codec_tags, str, codec_tag, ic, st);
>       if (codec_tag) {
>           uint32_t tag = strtol(codec_tag, &next, 0);
> @@ -1362,6 +1376,8 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>   
>       ds->dec_opts.flags |= DECODER_FLAG_BITEXACT * !!o->bitexact;
>   
> +    av_dict_set_int(&ds->decoder_opts, "apply_cropping", ds->apply_cropping, 0);
> +
>       /* Attached pics are sparse, therefore we would not want to delay their decoding
>        * till EOF. */
>       if (ist->st->disposition & AV_DISPOSITION_ATTACHED_PIC)
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index 12cca684b4..a31fa1be7f 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -1701,6 +1701,17 @@ static int configure_input_video_filter(FilterGraph *fg, AVFilterGraph *graph,
>       desc = av_pix_fmt_desc_get(ifp->format);
>       av_assert0(desc);
>   
> +    if ((ifp->opts.flags & IFILTER_FLAG_CROP) &&
> +        !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
> +        char crop_buf[64];
> +        snprintf(crop_buf, sizeof(crop_buf), "w=iw-%d-%d:h=ih-%d-%d",
> +                 ifp->opts.crop_left, ifp->opts.crop_right,
> +                 ifp->opts.crop_top, ifp->opts.crop_bottom);
> +        ret = insert_filter(&last_filter, &pad_idx, "crop", crop_buf);
> +        if (ret < 0)
> +            return ret;
> +    }

The crop filter supports hardware frames too.

> +
>       // TODO: insert hwaccel enabled filters like transpose_vaapi into the graph
>       ifp->displaymatrix_applied = 0;
>       if ((ifp->opts.flags & IFILTER_FLAG_AUTOROTATE) &&
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 910e4a336b..0c50ce16ba 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -1732,6 +1732,9 @@ const OptionDef options[] = {
>       { "autoscale",                  OPT_TYPE_BOOL,   OPT_PERSTREAM | OPT_EXPERT | OPT_OUTPUT,
>           { .off = OFFSET(autoscale) },
>           "automatically insert a scale filter at the end of the filter graph" },
> +    { "apply_cropping",             OPT_TYPE_BOOL,   OPT_PERSTREAM | OPT_EXPERT | OPT_INPUT,
> +        { .off = OFFSET(apply_cropping) },
> +        "apply frame cropping" },

Any reason why it can't be enabled by default?

>       { "fix_sub_duration_heartbeat", OPT_TYPE_BOOL,   OPT_VIDEO | OPT_EXPERT | OPT_PERSTREAM | OPT_OUTPUT,
>           { .off = OFFSET(fix_sub_duration_heartbeat) },
>           "set this video output stream to be a heartbeat stream for "
James Almer May 30, 2024, 1:04 a.m. UTC | #2
On 5/29/2024 10:01 PM, Lynne via ffmpeg-devel wrote:
> On 29/05/2024 23:46, James Almer wrote:
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   fftools/ffmpeg.h        |  7 +++++++
>>   fftools/ffmpeg_demux.c  | 16 ++++++++++++++++
>>   fftools/ffmpeg_filter.c | 11 +++++++++++
>>   fftools/ffmpeg_opt.c    |  3 +++
>>   4 files changed, 37 insertions(+)
>>
>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>> index fe75706afd..f908e16549 100644
>> --- a/fftools/ffmpeg.h
>> +++ b/fftools/ffmpeg.h
>> @@ -155,6 +155,7 @@ typedef struct OptionsContext {
>>       SpecifierOptList hwaccel_devices;
>>       SpecifierOptList hwaccel_output_formats;
>>       SpecifierOptList autorotate;
>> +    SpecifierOptList apply_cropping;
>>       /* output options */
>>       StreamMap *stream_maps;
>> @@ -239,6 +240,7 @@ enum IFilterFlags {
>>       IFILTER_FLAG_AUTOROTATE     = (1 << 0),
>>       IFILTER_FLAG_REINIT         = (1 << 1),
>>       IFILTER_FLAG_CFR            = (1 << 2),
>> +    IFILTER_FLAG_CROP           = (1 << 3),
>>   };
>>   typedef struct InputFilterOptions {
>> @@ -254,6 +256,11 @@ typedef struct InputFilterOptions {
>>        * accurate */
>>       AVRational          framerate;
>> +    unsigned crop_top;
>> +    unsigned crop_bottom;
>> +    unsigned crop_left;
>> +    unsigned crop_right;
>> +
>>       int                 sub2video_width;
>>       int                 sub2video_height;
>> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
>> index 1ca8d804ae..4178b8f840 100644
>> --- a/fftools/ffmpeg_demux.c
>> +++ b/fftools/ffmpeg_demux.c
>> @@ -66,6 +66,7 @@ typedef struct DemuxStream {
>>       int                      have_sub2video;
>>       int                      reinit_filters;
>>       int                      autorotate;
>> +    int                      apply_cropping;
>>       int                      wrap_correction_done;
>> @@ -1000,11 +1001,20 @@ int ist_filter_add(InputStream *ist, 
>> InputFilter *ifilter, int is_simple,
>>       ist->filters[ist->nb_filters - 1] = ifilter;
>>       if (ist->par->codec_type == AVMEDIA_TYPE_VIDEO) {
>> +        const 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 (ist->framerate.num > 0 && ist->framerate.den > 0) {
>>               opts->framerate = ist->framerate;
>>               opts->flags |= IFILTER_FLAG_CFR;
>>           } else
>>               opts->framerate = av_guess_frame_rate(d->f.ctx, ist->st, 
>> NULL);
>> +        if (sd && sd->size == sizeof(uint32_t) * 4) {
>> +            opts->crop_top    = AV_RL32(sd->data +  0);
>> +            opts->crop_bottom = AV_RL32(sd->data +  4);
>> +            opts->crop_left   = AV_RL32(sd->data +  8);
>> +            opts->crop_right  = AV_RL32(sd->data + 12);
>> +        }
>>       } else if (ist->par->codec_type == AVMEDIA_TYPE_SUBTITLE) {
>>           /* Compute the size of the canvas for the subtitles stream.
>>              If the subtitles codecpar has set a size, use it. 
>> Otherwise use the
>> @@ -1059,6 +1069,7 @@ int ist_filter_add(InputStream *ist, InputFilter 
>> *ifilter, int is_simple,
>>           return AVERROR(ENOMEM);
>>       opts->flags |= IFILTER_FLAG_AUTOROTATE * !!(ds->autorotate) |
>> +                   IFILTER_FLAG_CROP       * !!(ds->apply_cropping) |
>>                      IFILTER_FLAG_REINIT     * !!(ds->reinit_filters);
>>       return ds->sch_idx_dec;
>> @@ -1241,6 +1252,9 @@ static int ist_add(const OptionsContext *o, 
>> Demuxer *d, AVStream *st)
>>       ds->autorotate = 1;
>>       MATCH_PER_STREAM_OPT(autorotate, i, ds->autorotate, ic, st);
>> +    ds->apply_cropping = 1;
>> +    MATCH_PER_STREAM_OPT(apply_cropping, i, ds->apply_cropping, ic, st);
>> +
>>       MATCH_PER_STREAM_OPT(codec_tags, str, codec_tag, ic, st);
>>       if (codec_tag) {
>>           uint32_t tag = strtol(codec_tag, &next, 0);
>> @@ -1362,6 +1376,8 @@ static int ist_add(const OptionsContext *o, 
>> Demuxer *d, AVStream *st)
>>       ds->dec_opts.flags |= DECODER_FLAG_BITEXACT * !!o->bitexact;
>> +    av_dict_set_int(&ds->decoder_opts, "apply_cropping", 
>> ds->apply_cropping, 0);
>> +
>>       /* Attached pics are sparse, therefore we would not want to 
>> delay their decoding
>>        * till EOF. */
>>       if (ist->st->disposition & AV_DISPOSITION_ATTACHED_PIC)
>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
>> index 12cca684b4..a31fa1be7f 100644
>> --- a/fftools/ffmpeg_filter.c
>> +++ b/fftools/ffmpeg_filter.c
>> @@ -1701,6 +1701,17 @@ static int 
>> configure_input_video_filter(FilterGraph *fg, AVFilterGraph *graph,
>>       desc = av_pix_fmt_desc_get(ifp->format);
>>       av_assert0(desc);
>> +    if ((ifp->opts.flags & IFILTER_FLAG_CROP) &&
>> +        !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
>> +        char crop_buf[64];
>> +        snprintf(crop_buf, sizeof(crop_buf), "w=iw-%d-%d:h=ih-%d-%d",
>> +                 ifp->opts.crop_left, ifp->opts.crop_right,
>> +                 ifp->opts.crop_top, ifp->opts.crop_bottom);
>> +        ret = insert_filter(&last_filter, &pad_idx, "crop", crop_buf);
>> +        if (ret < 0)
>> +            return ret;
>> +    }
> 
> The crop filter supports hardware frames too.

It exports the values within the AVFrame fields. If that's acceptable 
then i can remove this check.

> 
>> +
>>       // TODO: insert hwaccel enabled filters like transpose_vaapi 
>> into the graph
>>       ifp->displaymatrix_applied = 0;
>>       if ((ifp->opts.flags & IFILTER_FLAG_AUTOROTATE) &&
>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>> index 910e4a336b..0c50ce16ba 100644
>> --- a/fftools/ffmpeg_opt.c
>> +++ b/fftools/ffmpeg_opt.c
>> @@ -1732,6 +1732,9 @@ const OptionDef options[] = {
>>       { "autoscale",                  OPT_TYPE_BOOL,   OPT_PERSTREAM | 
>> OPT_EXPERT | OPT_OUTPUT,
>>           { .off = OFFSET(autoscale) },
>>           "automatically insert a scale filter at the end of the 
>> filter graph" },
>> +    { "apply_cropping",             OPT_TYPE_BOOL,   OPT_PERSTREAM | 
>> OPT_EXPERT | OPT_INPUT,
>> +        { .off = OFFSET(apply_cropping) },
>> +        "apply frame cropping" },
> 
> Any reason why it can't be enabled by default?

Is is. Look at the line above the relevant MATCH_PER_STREAM_OPT line.

> 
>>       { "fix_sub_duration_heartbeat", OPT_TYPE_BOOL,   OPT_VIDEO | 
>> OPT_EXPERT | OPT_PERSTREAM | OPT_OUTPUT,
>>           { .off = OFFSET(fix_sub_duration_heartbeat) },
>>           "set this video output stream to be a heartbeat stream for "
> 
> 
> _______________________________________________
> 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".
Michael Niedermayer May 30, 2024, 10:57 p.m. UTC | #3
On Wed, May 29, 2024 at 06:46:31PM -0300, James Almer wrote:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  fftools/ffmpeg.h        |  7 +++++++
>  fftools/ffmpeg_demux.c  | 16 ++++++++++++++++
>  fftools/ffmpeg_filter.c | 11 +++++++++++
>  fftools/ffmpeg_opt.c    |  3 +++
>  4 files changed, 37 insertions(+)

seems to break fate here

--- ./tests/ref/vsynth/vsynth1-dnxhd-edge2-hr	2024-05-28 01:00:33.943039144 +0200
+++ tests/data/fate/vsynth1-dnxhd-edge2-hr	2024-05-31 00:56:06.577710365 +0200
@@ -1,4 +1,4 @@
 3ebeb52ae53a5b2ae4a0d90fa728c4fa *tests/data/fate/vsynth1-dnxhd-edge2-hr.dnxhd
 81920 tests/data/fate/vsynth1-dnxhd-edge2-hr.dnxhd
-1763637504f89c4e1a50a4de25c5e58a *tests/data/fate/vsynth1-dnxhd-edge2-hr.out.rawvideo
-stddev:   15.86 PSNR: 24.12 MAXDIFF:  157 bytes:  7603200/   760320
+dfa27c2e0f422c873c45d5abfcedf75f *tests/data/fate/vsynth1-dnxhd-edge2-hr.out.rawvideo
+stddev:   19.61 PSNR: 22.28 MAXDIFF:  170 bytes:  7603200/   760320
Test vsynth1-dnxhd-edge2-hr failed. Look at tests/data/fate/vsynth1-dnxhd-edge2-hr.err for details.
make: *** [tests/Makefile:311: fate-vsynth1-dnxhd-edge2-hr] Error 1

thx

[...]
Lynne May 31, 2024, 1:13 a.m. UTC | #4
On 30/05/2024 03:04, James Almer wrote:
> On 5/29/2024 10:01 PM, Lynne via ffmpeg-devel wrote:
>> On 29/05/2024 23:46, James Almer wrote:
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>>   fftools/ffmpeg.h        |  7 +++++++
>>>   fftools/ffmpeg_demux.c  | 16 ++++++++++++++++
>>>   fftools/ffmpeg_filter.c | 11 +++++++++++
>>>   fftools/ffmpeg_opt.c    |  3 +++
>>>   4 files changed, 37 insertions(+)
>>>
>>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>>> index fe75706afd..f908e16549 100644
>>> --- a/fftools/ffmpeg.h
>>> +++ b/fftools/ffmpeg.h
>>> @@ -155,6 +155,7 @@ typedef struct OptionsContext {
>>>       SpecifierOptList hwaccel_devices;
>>>       SpecifierOptList hwaccel_output_formats;
>>>       SpecifierOptList autorotate;
>>> +    SpecifierOptList apply_cropping;
>>>       /* output options */
>>>       StreamMap *stream_maps;
>>> @@ -239,6 +240,7 @@ enum IFilterFlags {
>>>       IFILTER_FLAG_AUTOROTATE     = (1 << 0),
>>>       IFILTER_FLAG_REINIT         = (1 << 1),
>>>       IFILTER_FLAG_CFR            = (1 << 2),
>>> +    IFILTER_FLAG_CROP           = (1 << 3),
>>>   };
>>>   typedef struct InputFilterOptions {
>>> @@ -254,6 +256,11 @@ typedef struct InputFilterOptions {
>>>        * accurate */
>>>       AVRational          framerate;
>>> +    unsigned crop_top;
>>> +    unsigned crop_bottom;
>>> +    unsigned crop_left;
>>> +    unsigned crop_right;
>>> +
>>>       int                 sub2video_width;
>>>       int                 sub2video_height;
>>> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
>>> index 1ca8d804ae..4178b8f840 100644
>>> --- a/fftools/ffmpeg_demux.c
>>> +++ b/fftools/ffmpeg_demux.c
>>> @@ -66,6 +66,7 @@ typedef struct DemuxStream {
>>>       int                      have_sub2video;
>>>       int                      reinit_filters;
>>>       int                      autorotate;
>>> +    int                      apply_cropping;
>>>       int                      wrap_correction_done;
>>> @@ -1000,11 +1001,20 @@ int ist_filter_add(InputStream *ist, 
>>> InputFilter *ifilter, int is_simple,
>>>       ist->filters[ist->nb_filters - 1] = ifilter;
>>>       if (ist->par->codec_type == AVMEDIA_TYPE_VIDEO) {
>>> +        const 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 (ist->framerate.num > 0 && ist->framerate.den > 0) {
>>>               opts->framerate = ist->framerate;
>>>               opts->flags |= IFILTER_FLAG_CFR;
>>>           } else
>>>               opts->framerate = av_guess_frame_rate(d->f.ctx, 
>>> ist->st, NULL);
>>> +        if (sd && sd->size == sizeof(uint32_t) * 4) {
>>> +            opts->crop_top    = AV_RL32(sd->data +  0);
>>> +            opts->crop_bottom = AV_RL32(sd->data +  4);
>>> +            opts->crop_left   = AV_RL32(sd->data +  8);
>>> +            opts->crop_right  = AV_RL32(sd->data + 12);
>>> +        }
>>>       } else if (ist->par->codec_type == AVMEDIA_TYPE_SUBTITLE) {
>>>           /* Compute the size of the canvas for the subtitles stream.
>>>              If the subtitles codecpar has set a size, use it. 
>>> Otherwise use the
>>> @@ -1059,6 +1069,7 @@ int ist_filter_add(InputStream *ist, 
>>> InputFilter *ifilter, int is_simple,
>>>           return AVERROR(ENOMEM);
>>>       opts->flags |= IFILTER_FLAG_AUTOROTATE * !!(ds->autorotate) |
>>> +                   IFILTER_FLAG_CROP       * !!(ds->apply_cropping) |
>>>                      IFILTER_FLAG_REINIT     * !!(ds->reinit_filters);
>>>       return ds->sch_idx_dec;
>>> @@ -1241,6 +1252,9 @@ static int ist_add(const OptionsContext *o, 
>>> Demuxer *d, AVStream *st)
>>>       ds->autorotate = 1;
>>>       MATCH_PER_STREAM_OPT(autorotate, i, ds->autorotate, ic, st);
>>> +    ds->apply_cropping = 1;
>>> +    MATCH_PER_STREAM_OPT(apply_cropping, i, ds->apply_cropping, ic, 
>>> st);
>>> +
>>>       MATCH_PER_STREAM_OPT(codec_tags, str, codec_tag, ic, st);
>>>       if (codec_tag) {
>>>           uint32_t tag = strtol(codec_tag, &next, 0);
>>> @@ -1362,6 +1376,8 @@ static int ist_add(const OptionsContext *o, 
>>> Demuxer *d, AVStream *st)
>>>       ds->dec_opts.flags |= DECODER_FLAG_BITEXACT * !!o->bitexact;
>>> +    av_dict_set_int(&ds->decoder_opts, "apply_cropping", 
>>> ds->apply_cropping, 0);
>>> +
>>>       /* Attached pics are sparse, therefore we would not want to 
>>> delay their decoding
>>>        * till EOF. */
>>>       if (ist->st->disposition & AV_DISPOSITION_ATTACHED_PIC)
>>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
>>> index 12cca684b4..a31fa1be7f 100644
>>> --- a/fftools/ffmpeg_filter.c
>>> +++ b/fftools/ffmpeg_filter.c
>>> @@ -1701,6 +1701,17 @@ static int 
>>> configure_input_video_filter(FilterGraph *fg, AVFilterGraph *graph,
>>>       desc = av_pix_fmt_desc_get(ifp->format);
>>>       av_assert0(desc);
>>> +    if ((ifp->opts.flags & IFILTER_FLAG_CROP) &&
>>> +        !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
>>> +        char crop_buf[64];
>>> +        snprintf(crop_buf, sizeof(crop_buf), "w=iw-%d-%d:h=ih-%d-%d",
>>> +                 ifp->opts.crop_left, ifp->opts.crop_right,
>>> +                 ifp->opts.crop_top, ifp->opts.crop_bottom);
>>> +        ret = insert_filter(&last_filter, &pad_idx, "crop", crop_buf);
>>> +        if (ret < 0)
>>> +            return ret;
>>> +    }
>>
>> The crop filter supports hardware frames too.
> 
> It exports the values within the AVFrame fields. If that's acceptable 
> then i can remove this check.
Sure, that's acceptable. If anything, it would help get better support 
for AVFrame's crop fields through the codebase.
Anton Khirnov June 25, 2024, 10:19 a.m. UTC | #5
Quoting James Almer (2024-05-29 23:46:31)
> @@ -254,6 +256,11 @@ typedef struct InputFilterOptions {
>       * accurate */
>      AVRational          framerate;
>  
> +    unsigned crop_top;
> +    unsigned crop_bottom;
> +    unsigned crop_left;
> +    unsigned crop_right;

breaks vertical alignment

> +
>      int                 sub2video_width;
>      int                 sub2video_height;
>  
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index 1ca8d804ae..4178b8f840 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -66,6 +66,7 @@ typedef struct DemuxStream {
>      int                      have_sub2video;
>      int                      reinit_filters;
>      int                      autorotate;
> +    int                      apply_cropping;
>  
>  
>      int                      wrap_correction_done;
> @@ -1000,11 +1001,20 @@ int ist_filter_add(InputStream *ist, InputFilter *ifilter, int is_simple,
>      ist->filters[ist->nb_filters - 1] = ifilter;
>  
>      if (ist->par->codec_type == AVMEDIA_TYPE_VIDEO) {
> +        const 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 (ist->framerate.num > 0 && ist->framerate.den > 0) {
>              opts->framerate = ist->framerate;
>              opts->flags |= IFILTER_FLAG_CFR;
>          } else
>              opts->framerate = av_guess_frame_rate(d->f.ctx, ist->st, NULL);
> +        if (sd && sd->size == sizeof(uint32_t) * 4) {

better make it <= to account for future extensions, e.g. we might want
to add sub-pixel resolution in there

> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 910e4a336b..0c50ce16ba 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -1732,6 +1732,9 @@ const OptionDef options[] = {
>      { "autoscale",                  OPT_TYPE_BOOL,   OPT_PERSTREAM | OPT_EXPERT | OPT_OUTPUT,
>          { .off = OFFSET(autoscale) },
>          "automatically insert a scale filter at the end of the filter graph" },
> +    { "apply_cropping",             OPT_TYPE_BOOL,   OPT_PERSTREAM | OPT_EXPERT | OPT_INPUT,
> +        { .off = OFFSET(apply_cropping) },
> +        "apply frame cropping" },

Apply container-level cropping (in addition to codec-level cropping)

Also, missing docs.
Anton Khirnov July 2, 2024, 5:55 p.m. UTC | #6
Quoting James Almer (2024-07-02 18:49:36)
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  doc/ffmpeg.texi         | 16 ++++++++++++++++
>  fftools/ffmpeg.h        | 15 +++++++++++++++
>  fftools/ffmpeg_demux.c  | 26 ++++++++++++++++++++++++++
>  fftools/ffmpeg_filter.c | 10 ++++++++++
>  fftools/ffmpeg_opt.c    | 25 +++++++++++++++++++++++++
>  5 files changed, 92 insertions(+)
> 
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index f25f6192eb..f75ed681cf 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -1262,6 +1262,22 @@ disabled, all output frames of filter graph might not be in the same resolution
>  and may be inadequate for some encoder/muxer. Therefore, it is not recommended
>  to disable it unless you really know what you are doing.
>  Disable autoscale at your own risk.
> +
> +@item -apply_cropping

+@item -apply_cropping[:@var{stream_specifier}] @var{source} (@emph{input,per-stream})


> +Automatically crop the video according to file metadata. Default is @emph{all}.
                               ^
                        after decoding
> +
> +@table @option
> +@item none (0)
> +Don't apply any cropping metadata.
> +@item all (1)
> +Apply both codec and container level croppping. This is the default mode.
> +@item codec (2)
> +Apply codec level croppping.
> +@item container (3)
> +Apply container level croppping.
> +
> +@end table
> +

Also, this should probably be in the advanced section, since it's marked
as OPT_EXPERT.

> @@ -715,6 +729,7 @@ AVDictionary *strip_specifiers(const AVDictionary *dict);
>  int find_codec(void *logctx, const char *name,
>                 enum AVMediaType type, int encoder, const AVCodec **codec);
>  int parse_and_set_vsync(const char *arg, int *vsync_var, int file_idx, int st_idx, int is_global);
> +int parse_and_set_cropping(const char *arg, int *out);

What's the point of this being in a separate file when it's only used
from ffmpeg_demux?
James Almer July 2, 2024, 6:43 p.m. UTC | #7
On 7/2/2024 2:55 PM, Anton Khirnov wrote:
> Quoting James Almer (2024-07-02 18:49:36)
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   doc/ffmpeg.texi         | 16 ++++++++++++++++
>>   fftools/ffmpeg.h        | 15 +++++++++++++++
>>   fftools/ffmpeg_demux.c  | 26 ++++++++++++++++++++++++++
>>   fftools/ffmpeg_filter.c | 10 ++++++++++
>>   fftools/ffmpeg_opt.c    | 25 +++++++++++++++++++++++++
>>   5 files changed, 92 insertions(+)
>>
>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>> index f25f6192eb..f75ed681cf 100644
>> --- a/doc/ffmpeg.texi
>> +++ b/doc/ffmpeg.texi
>> @@ -1262,6 +1262,22 @@ disabled, all output frames of filter graph might not be in the same resolution
>>   and may be inadequate for some encoder/muxer. Therefore, it is not recommended
>>   to disable it unless you really know what you are doing.
>>   Disable autoscale at your own risk.
>> +
>> +@item -apply_cropping
> 
> +@item -apply_cropping[:@var{stream_specifier}] @var{source} (@emph{input,per-stream})

Ok.

> 
> 
>> +Automatically crop the video according to file metadata. Default is @emph{all}.
>                                 ^
>                          after decoding

Ok.

>> +
>> +@table @option
>> +@item none (0)
>> +Don't apply any cropping metadata.
>> +@item all (1)
>> +Apply both codec and container level croppping. This is the default mode.
>> +@item codec (2)
>> +Apply codec level croppping.
>> +@item container (3)
>> +Apply container level croppping.
>> +
>> +@end table
>> +
> 
> Also, this should probably be in the advanced section, since it's marked
> as OPT_EXPERT.

So are autoscale and autorotate, which are also outside the advanced 
section. But i can move it if you prefer.

> 
>> @@ -715,6 +729,7 @@ AVDictionary *strip_specifiers(const AVDictionary *dict);
>>   int find_codec(void *logctx, const char *name,
>>                  enum AVMediaType type, int encoder, const AVCodec **codec);
>>   int parse_and_set_vsync(const char *arg, int *vsync_var, int file_idx, int st_idx, int is_global);
>> +int parse_and_set_cropping(const char *arg, int *out);
> 
> What's the point of this being in a separate file when it's only used
> from ffmpeg_demux?

Copy-paste implementation from parse_and_set_vsync(). Will move.
Anton Khirnov July 2, 2024, 7 p.m. UTC | #8
Quoting James Almer (2024-07-02 20:43:59)
> On 7/2/2024 2:55 PM, Anton Khirnov wrote:
> >> +
> >> +@table @option
> >> +@item none (0)
> >> +Don't apply any cropping metadata.
> >> +@item all (1)
> >> +Apply both codec and container level croppping. This is the default mode.
> >> +@item codec (2)
> >> +Apply codec level croppping.
> >> +@item container (3)
> >> +Apply container level croppping.
> >> +
> >> +@end table
> >> +
> > 
> > Also, this should probably be in the advanced section, since it's marked
> > as OPT_EXPERT.
> 
> So are autoscale and autorotate, which are also outside the advanced 
> section. But i can move it if you prefer.

Please do.

Patch ok otherwise.
diff mbox series

Patch

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index fe75706afd..f908e16549 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -155,6 +155,7 @@  typedef struct OptionsContext {
     SpecifierOptList hwaccel_devices;
     SpecifierOptList hwaccel_output_formats;
     SpecifierOptList autorotate;
+    SpecifierOptList apply_cropping;
 
     /* output options */
     StreamMap *stream_maps;
@@ -239,6 +240,7 @@  enum IFilterFlags {
     IFILTER_FLAG_AUTOROTATE     = (1 << 0),
     IFILTER_FLAG_REINIT         = (1 << 1),
     IFILTER_FLAG_CFR            = (1 << 2),
+    IFILTER_FLAG_CROP           = (1 << 3),
 };
 
 typedef struct InputFilterOptions {
@@ -254,6 +256,11 @@  typedef struct InputFilterOptions {
      * accurate */
     AVRational          framerate;
 
+    unsigned crop_top;
+    unsigned crop_bottom;
+    unsigned crop_left;
+    unsigned crop_right;
+
     int                 sub2video_width;
     int                 sub2video_height;
 
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 1ca8d804ae..4178b8f840 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -66,6 +66,7 @@  typedef struct DemuxStream {
     int                      have_sub2video;
     int                      reinit_filters;
     int                      autorotate;
+    int                      apply_cropping;
 
 
     int                      wrap_correction_done;
@@ -1000,11 +1001,20 @@  int ist_filter_add(InputStream *ist, InputFilter *ifilter, int is_simple,
     ist->filters[ist->nb_filters - 1] = ifilter;
 
     if (ist->par->codec_type == AVMEDIA_TYPE_VIDEO) {
+        const 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 (ist->framerate.num > 0 && ist->framerate.den > 0) {
             opts->framerate = ist->framerate;
             opts->flags |= IFILTER_FLAG_CFR;
         } else
             opts->framerate = av_guess_frame_rate(d->f.ctx, ist->st, NULL);
+        if (sd && sd->size == sizeof(uint32_t) * 4) {
+            opts->crop_top    = AV_RL32(sd->data +  0);
+            opts->crop_bottom = AV_RL32(sd->data +  4);
+            opts->crop_left   = AV_RL32(sd->data +  8);
+            opts->crop_right  = AV_RL32(sd->data + 12);
+        }
     } else if (ist->par->codec_type == AVMEDIA_TYPE_SUBTITLE) {
         /* Compute the size of the canvas for the subtitles stream.
            If the subtitles codecpar has set a size, use it. Otherwise use the
@@ -1059,6 +1069,7 @@  int ist_filter_add(InputStream *ist, InputFilter *ifilter, int is_simple,
         return AVERROR(ENOMEM);
 
     opts->flags |= IFILTER_FLAG_AUTOROTATE * !!(ds->autorotate) |
+                   IFILTER_FLAG_CROP       * !!(ds->apply_cropping) |
                    IFILTER_FLAG_REINIT     * !!(ds->reinit_filters);
 
     return ds->sch_idx_dec;
@@ -1241,6 +1252,9 @@  static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
     ds->autorotate = 1;
     MATCH_PER_STREAM_OPT(autorotate, i, ds->autorotate, ic, st);
 
+    ds->apply_cropping = 1;
+    MATCH_PER_STREAM_OPT(apply_cropping, i, ds->apply_cropping, ic, st);
+
     MATCH_PER_STREAM_OPT(codec_tags, str, codec_tag, ic, st);
     if (codec_tag) {
         uint32_t tag = strtol(codec_tag, &next, 0);
@@ -1362,6 +1376,8 @@  static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
 
     ds->dec_opts.flags |= DECODER_FLAG_BITEXACT * !!o->bitexact;
 
+    av_dict_set_int(&ds->decoder_opts, "apply_cropping", ds->apply_cropping, 0);
+
     /* Attached pics are sparse, therefore we would not want to delay their decoding
      * till EOF. */
     if (ist->st->disposition & AV_DISPOSITION_ATTACHED_PIC)
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 12cca684b4..a31fa1be7f 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -1701,6 +1701,17 @@  static int configure_input_video_filter(FilterGraph *fg, AVFilterGraph *graph,
     desc = av_pix_fmt_desc_get(ifp->format);
     av_assert0(desc);
 
+    if ((ifp->opts.flags & IFILTER_FLAG_CROP) &&
+        !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
+        char crop_buf[64];
+        snprintf(crop_buf, sizeof(crop_buf), "w=iw-%d-%d:h=ih-%d-%d",
+                 ifp->opts.crop_left, ifp->opts.crop_right,
+                 ifp->opts.crop_top, ifp->opts.crop_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
     ifp->displaymatrix_applied = 0;
     if ((ifp->opts.flags & IFILTER_FLAG_AUTOROTATE) &&
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 910e4a336b..0c50ce16ba 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1732,6 +1732,9 @@  const OptionDef options[] = {
     { "autoscale",                  OPT_TYPE_BOOL,   OPT_PERSTREAM | OPT_EXPERT | OPT_OUTPUT,
         { .off = OFFSET(autoscale) },
         "automatically insert a scale filter at the end of the filter graph" },
+    { "apply_cropping",             OPT_TYPE_BOOL,   OPT_PERSTREAM | OPT_EXPERT | OPT_INPUT,
+        { .off = OFFSET(apply_cropping) },
+        "apply frame cropping" },
     { "fix_sub_duration_heartbeat", OPT_TYPE_BOOL,   OPT_VIDEO | OPT_EXPERT | OPT_PERSTREAM | OPT_OUTPUT,
         { .off = OFFSET(fix_sub_duration_heartbeat) },
         "set this video output stream to be a heartbeat stream for "