>De : ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> 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 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".