[FFmpeg-devel] [PATCHv2 3/4] avutil/pixdesc: add AV_PIX_FMT_FLAG_ALPHA to AV_PIX_FMT_PAL8

[Marton Balint]

On Sun, 22 Apr 2018, wm4 wrote:

> On Sun, 22 Apr 2018 13:24:11 +0200 (CEST)
> Marton Balint <cus at passwd.hu> wrote:
>
>> On Fri, 20 Apr 2018, wm4 wrote:
>> 
>> > On Thu, 19 Apr 2018 23:25:03 +0200
>> > Marton Balint <cus at passwd.hu> wrote:
>> > 
>> >> Signed-off-by: Marton Balint <cus at passwd.hu>
>> >> ---
>> >>  doc/APIchanges            | 3 +++
>> >>  libavutil/pixdesc.c       | 3 +--
>> >>  libavutil/pixdesc.h       | 8 ++------
>> >>  libavutil/tests/pixdesc.c | 4 ----
>> >>  libavutil/version.h       | 2 +-
>> >>  5 files changed, 7 insertions(+), 13 deletions(-)
>> >> 
>> >> diff --git a/doc/APIchanges b/doc/APIchanges
>> >> index 4f6ac2a031..d9b457e080 100644
>> >> --- a/doc/APIchanges
>> >> +++ b/doc/APIchanges
>> >> @@ -15,6 +15,9 @@ libavutil:     2017-10-21
>> >>
>> >>  API changes, most recent first:
>> >> 
>> >> +2018-04-xx - xxxxxxxxxx - lavu 56.16.100 - pixdesc.h
>> >> +  Add AV_PIX_FMT_FLAG_ALPHA to AV_PIX_FMT_PAL8.
>> >> + 
>> 
>> [..]
>> 
>> >
>> > Probably fine. While I like it, we also have to be careful about the
>> > consequences. Does it change FATE or the results of that pixfmt choosing
>> > function, avcodec_find_best_pix_fmt_of_list()? 
>> 
>> Fate passes. I am not sure about avcodec_find_best_pix_fmt_of_list(), but 
>> since pixdesc_has_alpha() in avutil/pixdesc.c already considered PAL8 as a 
>> format with alpha, I don't think it changes.
>
> Oh, interesting point. So this whole discussion is moot anyway, since
> it always suggested RGBA when converting PAL8 to a non-paletted RGB
> format?

Only if has_alpha was true when the user called 
avcodec_find_best_pix_fmt_of_list() or av_find_best_pix_fmt_of_2().

For avfiltergraph format negotiation or for ffmpeg.c automatic codec pixel 
format selection, pal8 was not considered as a format with alpha 
(nb_components % 2 == 0 check was used), so they called 
av_find_best_pix_fmt_of_2() with has_alpha=false, therefore alpha loss was 
not penalized. A subsequent patch takes care of that.

Regards,
Marton