On Tue, Jun 25, 2024 at 3:12 PM Anton Khirnov <an...@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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
