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

[James Almer <jamrial@gmail.com>]

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