Wrong decision of pixel format in filtergraph

[Muhammad Faiz]

Summary of the bug:
How to reproduce:

% ffmpeg -loglevel verbose -filter_complex "allrgb=d=0.1, lutyuv, format=yuv444p12be" -f null -

[Parsed_allrgb_0 @ 0x1b29f40] size:4096x4096 rate:25/1 duration:0.100000 sar:1/1
[auto_scaler_0 @ 0x1bad300] w:iw h:ih flags:'bilinear' interl:0
[Parsed_lutyuv_1 @ 0x1b2ab40] auto-inserting filter 'auto_scaler_0' between the filter 'Parsed_allrgb_0' and the filter 'Parsed_lutyuv_1'
[auto_scaler_1 @ 0x1bae9c0] w:iw h:ih flags:'bilinear' interl:0
[Parsed_format_2 @ 0x1b2a840] auto-inserting filter 'auto_scaler_1' between the filter 'Parsed_lutyuv_1' and the filter 'Parsed_format_2'
[auto_scaler_0 @ 0x1bad300] w:4096 h:4096 fmt:rgb24 sar:1/1 -> w:4096 h:4096 fmt:yuv444p sar:1/1 flags:0x2
[auto_scaler_1 @ 0x1bae9c0] w:4096 h:4096 fmt:yuv444p sar:1/1 -> w:4096 h:4096 fmt:yuv444p12be sar:1/1 flags:0x2

Note that the conversion is: rgb24 => yuv444p => yuv444p12be.
I guess the correct conversion is: rgb24 => yuv444p12le => yuv444p12be (lutyuv supports yuv444p12le).

[Carl Eugen Hoyos]

Why do you think that another conversion is better? Is there a formal argumentation?

I am not convinced this can be fixed, there definitely are (similar) cases where you have to use the format filter to fix the conversion.

[Muhammad Faiz]

A more severe example

% ffmpeg -loglevel verbose -filter_complex "aevalsrc=0:d=0.1, showcqt, format=bgr24" -f null -

[Parsed_showcqt_1 @ 0xe63ec0] auto-inserting filter 'auto_resampler_0' between the filter 'Parsed_aevalsrc_0' and the filter 'Parsed_showcqt_1'
[auto_scaler_0 @ 0xe67980] w:iw h:ih flags:'bilinear' interl:0
[Parsed_format_2 @ 0xe64f80] auto-inserting filter 'auto_scaler_0' between the filter 'Parsed_showcqt_1' and the filter 'Parsed_format_2'
[Parsed_aevalsrc_0 @ 0xe634c0] sample_rate:44100 chlayout:mono duration:100000
[auto_resampler_0 @ 0xe67b80] ch:1 chl:mono fmt:dblp r:44100Hz -> ch:2 chl:stereo fmt:flt r:44100Hz
[Parsed_showcqt_1 @ 0xe63ec0] video: 1920x1080 yuv420p 25/1 fps, bar_h = 524, axis_h = 32, sono_h = 524.
[Parsed_showcqt_1 @ 0xe63ec0] fft_len = 8192, cqt_len = 1920.
[Parsed_showcqt_1 @ 0xe63ec0] nb_cqt_coeffs = 44508.
[Parsed_showcqt_1 @ 0xe63ec0] audio: 44100 Hz, step = 294.
[auto_scaler_0 @ 0xe67980] w:1920 h:1080 fmt:yuv420p sar:1/1 -> w:1920 h:1080 fmt:bgr24 sar:1/1 flags:0x2

While showcqt actually supports rgb24, the filtergraph chooses yuv420p -> bgr24 rather than rgb24 -> bgr24. This is a problem because supported pixel/sample format is currently undocumented in filters' documentation. If it is documented, a user for example can carefully insert format=rgb24 before format=bgr24.

[Carl Eugen Hoyos]

(Why don't you try to answer my questions?)

The second example looks like something that could be improved (the system tries to find a best pix_fmt between aevalsrc and showcqt if showcqt and format do not have a common pix_fmt, this seems not ideal since aevalsrc has no pix_fmt) but it is unrelated to your first example.
In any case, I wonder why your past tickets (that were fixed) were valid and showed all necessary information while you decided that this ticket does not deserve to show all necessary information.

[Muhammad Faiz]

OK,
for lutyuv:
actual: rgb24 -> yuv444p -> yuv444p12be
ideal: rgb24 -> yuv444p12le -> yuv444p12be
The ideal pixfmt should be yuv444p12le because it is exactly yuv444p12be with different endianness. (OK, one can argue that yuv444p is closer to rgb24).

for showcqt:
actual: none -> yuv420p -> bgr24
ideal: none -> rgb24 -> bgr24
The ideal pixfmt should be rgb24 because it is exactly bgr24 with different red-green-blue ordering. And no one can argue that yuv420p is closer to 'none'.

The root cause as you stated is similar. The pixfmt decision on filter is solely based on input filters when there are no common pixfmts.

I posted lutyuv example because I was retrying to fix https://ffmpeg.org/pipermail/ffmpeg-devel/2016-June/195941.html

[Carl Eugen Hoyos]

Replying to mfcc64:

The root cause as you stated is similar.

I don't think they are similar: The first case seems a matter of taste, I don't think you were able to explain formally why one is better, the second case is a question of knowing what comes further down the filter-chain. The second case cannot be fixed in general but the specific issue of a filter not providing any sample format and the later ones not having a format in common but very similar ones maybe fixable.

I posted lutyuv example because I was retrying to fix https://ffmpeg.org/pipermail/ffmpeg-devel/2016-June/195941.html

I believe you have to fix this in the fate subsystem, your patch seems to make a lot of sense (apart from the fact that you want to improve big-endian systems while being unable to test them).

[Muhammad Faiz]

After investigating fate-run.sh, on pixfmts() actually the tested filter is guarded by format filter, so I try this:

% ffmpeg -loglevel verbose -filter_complex "allrgb=d=0.1, format=yuv444p12be, lutyuv, format=yuv444p12be" -f null -

[Parsed_allrgb_0 @ 0x1e84440] size:4096x4096 rate:25/1 duration:0.100000 sar:1/1
[auto_scaler_0 @ 0x1f080c0] w:iw h:ih flags:'bilinear' interl:0
[Parsed_format_1 @ 0x1e850c0] auto-inserting filter 'auto_scaler_0' between the filter 'Parsed_allrgb_0' and the filter 'Parsed_format_1'
[auto_scaler_1 @ 0x1f096c0] w:iw h:ih flags:'bilinear' interl:0
[Parsed_lutyuv_2 @ 0x1e85b00] auto-inserting filter 'auto_scaler_1' between the filter 'Parsed_format_1' and the filter 'Parsed_lutyuv_2'
[auto_scaler_2 @ 0x1f0ad00] w:iw h:ih flags:'bilinear' interl:0
[Parsed_format_3 @ 0x1e7fb40] auto-inserting filter 'auto_scaler_2' between the filter 'Parsed_lutyuv_2' and the filter 'Parsed_format_3'
[auto_scaler_0 @ 0x1f080c0] w:4096 h:4096 fmt:rgb24 sar:1/1 -> w:4096 h:4096 fmt:yuv444p12be sar:1/1 flags:0x2
[auto_scaler_1 @ 0x1f096c0] w:4096 h:4096 fmt:yuv444p12be sar:1/1 -> w:4096 h:4096 fmt:yuv444p16le sar:1/1 flags:0x2
[auto_scaler_2 @ 0x1f0ad00] w:4096 h:4096 fmt:yuv444p16le sar:1/1 -> w:4096 h:4096 fmt:yuv444p12be sar:1/1 flags:0x2

The result is unexpected: yuv444p12be -> yuv444p16le -> yuv444p12be.
The ideal should be: yuv444p12be -> yuv444p12le -> yuv444p12be.