Use "force_full_range" property to override the color range

[bmatherly]

Parital fix for: https://forum.shotcut.org/t/23-04-color-range-regression/38609

[ddennedy]

I have been working on this (I updated #1434 about that), and this is not the problem. The problem is actually in theory old if you had a source that is yuv422. The problem is that now the player is configuring the consumer for yuv420p, and most sources are yuv420p. Now, avcolor_space filter thinks it has nothing to do, so no range conversion is performed. color_space is a documented AVOption, and avformat producer applies all MLT properties that is also an AVOption to the context objects in apply_properties().

[bmatherly]

Between this PR and https://github.com/mltframework/mlt/pull/912, I was able to get the expected behavior. But it sounds like you might have a handle on a more definitive fix. Thanks for looking into it.

I propose that we move this issue to "blocking" for the release.

[bmatherly]

Now, avcolor_space filter thinks it has nothing to do, so no range conversion is performed

That is also my conclusion. Maybe this shameful workaround could get us by: https://github.com/mltframework/mlt/pull/912#pullrequestreview-1412223649

[ddennedy]

I decided I prefer the semantics of using "color_range" since that is what we also use on the consumer, and it is an AVOption: https://mltframework.org/plugins/ProducerAvformat/#color_range

[bmatherly]

Thanks Dan, https://github.com/mltframework/mlt/commit/97dcafde3ebb6a196cc102d6ae5228216a91d60b is working for me in my tests. And I agree with the approach.

Some comments:

The documentation for color range lists string values (unknown, tv, pc, unspecified, mpeg, jpeg). But in the code we are comparing to a number.

full_range = mlt_properties_get_int(properties, "color_range") == 2;

I think our implementation should match the documentation. If you tell me which is preferred, I can propose a patch to make them consistent.

Would you like me to propose a patch to mark force_full_range as deprecated?

[ddennedy]

View the output for ffmpeg -h full and search for color_range. It accepts both integers and strings. Our generated documentation says that, but it does not know how to show the integer options. But you are correct that my implementation is not working with the string values! I do not like the idea of comparing a property with several strings for every frame. So, I pushed a change to slightly refactor this into a property change listener instead of being inline within producer_get_image(). Then, "color_range" is handled as an AVOption and producer private state updated from the AVCodecContext. This way, it is only evaluated on property change, and the values - string or numeric - handled by FFmpeg. This property listener will probably become more generally useful such as as to always update a corresponding AVOption, but I want to keep the amount change right now small. I would have also liked to get rid of the redundant producer_avformat.full_range and just use AVCodecContext.color_range, but the same reason.