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

[Paul B Mahol]

On Tue, Jun 25, 2024 at 3:12 PM Anton Khirnov <anton at khirnov.net> wrote:

> Quoting James Almer (2024-06-25 14:38:58)
> > On 6/25/2024 7:25 AM, Anton Khirnov wrote:
> > > Quoting James Almer (2024-05-31 01:22:51)
> > >> @@ -1000,11 +1001,21 @@ 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->par->coded_side_data,
> > >> +
>  ist->par->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);
> > >> +            opts->flags      |= IFILTER_FLAG_CROP;
> > >> +        }
> > >>       } 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
> > >> @@ -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);
> > >
> > > If I'm reading it right, this new option now applies only to decoder
> > > cropping (breaking syntax, because AVOptions always take an argument),
> > > while container cropping is always applied unconditionally.
> > >
> > > That seems wrong.
> >
> > Yeah, for some reason i missed a "* !!ds->apply_cropping" next to the
> > IFILTER_FLAG_CROP above. With it container cropping is applied depending
> > on the value of apply_cropping.
> >
> > And how can i work around the ffmpeg option shadowing the avcodec one of
> > the same name? Using a different name for container cropping option
> > exclusively in ffmpeg is not really nice for the user. They either care
> > about cropping no matter the source, or no cropping.
>
> I expect plenty of broken files from various sources where the user will
> want to selectively ignore either codec or container cropping.
> So we could make this an enum option, with values
> none/all/codec/container, mapping to 0/1/2/3 respectively. That should
> keep compatibility with current syntax.
>

Enum? Why not flags?


>
> --
> Anton Khirnov
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>