>De : ffmpeg-devel <[email protected]> De la part de Niklas Haas
>Envoyé : jeudi 4 avril 2024 14:44
>
>> >> @@ -735,6 +736,9 @@ static int filter_frame(AVFilterLink *link, AVFrame
>> >> *in)
>> >> return res;
>> >> }
>> >>
>> >> + out->colorspace = s->out_csp;
>> >> + outlink->color_range = s->user_rng != AVCOL_RANGE_UNSPECIFIED ?
>> >> s->user_rng : in->color_range;
>> >> + out->color_range = outlink->color_range;
>> >
>> >Changing outlink dynamically like this seems not correct to me (what if the
>> >next filter in the chain only supports one color range?). Changing the
>> >range on the fly would at the minimum require reconfiguring the filter
>> >graph, and >>possibly insertion of more auto-scale filters.
>> This is the kind of questioning I had when working on this issue. This seems
>> very annoying and overly complex for a very uncommon scenario; is it even
>> possible within the filter to ask for a reconfiguration of the filter graph ?
>
>No, reconfiguring the filter graph is (currently) the API user's
>responsibility. (See fftools/ffmpeg_filter.c for an example)
>
>vf_buffersrc even warns you if you try and change colorspace properties
>mid-stream, and for good reason - IMO there is no general expectation that
>filters should be able to handle dynamically changing colorspace properties.
>(This is >a feature, not a bug)
>
>There *are* some filters that handle dynamically changing input properties
>(e.g. scale, zscale, libplacebo), but these are the exception rather than the
>norm, and it's only because they're already written in a way that can
>trivially >handle dynamic conversions.
>
>If it's difficult to do from within vf_colorspace, there's no need to do so.
>Feel free to assume that frame->colorspace == inlink->colorspace == constant.
>(Ditto color_range)
Thank you, this is pretty clear. I agree dynamic change of color range sounds
weird and I am pleased it can be assumed constant.
In my patch, it means the problematic "outlink->color_range = xxx" you pointed
at can be dropped. Nice.
>>
>> >The logic that is used in the other YUV negotiation aware filters is to
>> >just set `out->colorspace = outlink->colorspace` and `out->color_range =
>> >outlink->color_range`, since the link properties are authoritative.
>> This is the kind of easy logic I tried at the beginning. Some water has
>> flowed under the bridge, notably b89ee26539, but I just tried at the moment
>> (with current code) an easy patch with just these two lines, and it still
>> does not >work as "I" expected.
>> More specifically:
>> When running: ./ffprobe -f lavfi -i
>> yuvtestsrc,setparams=color_primaries=bt470bg:color_trc=smpte170m:color
>> space=bt470bg,colorspace=bt709:range=tv,scale,showinfo
>> At the entry of filter_frame(), the outlink values are incorrect:
>> colorspace = AVCOL_SPC_BT470BG
>> And color_range = AVCOL_RANGE_UNSPECIFIED So, this is why I introduced
>> the negotiation for the colorspace to early set and persist this outlink
>> property.
>> But for the range, I am bit confused with the documentation, it is not clear
>> to me, but possibly it is expected to pass through? so it cannot be
>> negotiated, so I am sticked if the outlink range cannot be changed
>> dynamically...
>
>Note: passing through the range untouched *is* the default behavior (via
>ff_default_query_formats). So I think the correct logic can be condensed into
>just:
>
>if (out_rng != UNSPEC) {
> ret = set_common_color_ranges(make_singleton(out_rng));
> if (ret < 0)
> return ret;
>}
>
>That way, if the user passes unspec, it's guaranteed that the input range ==
>output_range (but with no preference for any specific range); but if the user
>passes a specific range, both the input and output of the filter are forced to
>be >this range.
Well, this is where I still not feel confident. Dynamic input range is not
expected but somewhat still supported.
First thing: in my understanding, the colorspace filter is aimed at converting
from any input range to the desired output range (if specified), and I think my
patch is ok with the current
"ff_formats_ref(ff_make_formats_list_singleton(s->user_rng),
&outlink->incfg.color_ranges)". I don't think they are issues against it, so I
will keep it that way unless you object.
Second thing: I understand the default behaviour is to pass through (I
mean/guess dynamically) the range, but it is not what I experience. To test
this, my patch serie includes a first patch to make setparams support timeline
and here it is when running a "dynamic input range input":
ffmpeg -f lavfi -i yuvtestsrc -vf
"setparams=color_primaries=bt470bg:color_trc=smpte170m:colorspace=bt470bg:range=unknown,
setparams=range=pc:enable='between(n,1,2)',
setparams=range=tv:enable='between(n,2,3)',
colorspace=bt709,scale,showinfo" -f null -frames 3 - 2>&1|awk "/color_/ {print
\$4 \" \" \$5}"
Current master (solely patched for timeline support in setparams):
color_range:tv color_space:bt470bg
color_range:tv color_space:bt470bg
color_range:tv color_space:bt470bg
My current patch:
color_range:unknown color_space:bt709
color_range:pc color_space:bt709
color_range:tv color_space:bt709
Release tag 6.1 (solely patched for timeline support in setparams):
color_range:unknown color_space:bt709
color_range:pc color_space:bt709
color_range:tv color_space:bt709
My current patch without the problematic "outlink->color_range = xxx"
color_range:tv color_space:bt709
color_range:tv color_space:bt709
color_range:tv color_space:bt709
So, I can remove the problematic outlink change, but it causes more or less a
subtle <<regression>>; I don't think it is the good word for it since what has
been said above...
=> If I read you correctly, I really have to drop this problematic outlink
setting, and the resulting slight change in the colorspace filter behaviour is
okay.
=> At the end, if I drop the outlink setting, clear the nits and update the
commit msg to remove the "dynamic input range scenario", I would be on the
right track, so this is to be my next version.
>Hopefully that clears up some confusion?
Sure! Some definite confirmations remain but thank you already for all this.
Nicolas
_______________________________________________
ffmpeg-devel mailing list
[email protected]
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
[email protected] with subject "unsubscribe".
