[FFmpeg-devel] [PATCH]Fix pix_fmt_info for yuva420

[Stefano Sabatini]

On date Monday 2012-01-16 15:35:32 +0100, Carl Eugen Hoyos encoded:
> Hi!
> 
> Attached patch fixes pix_fmt selection for yuva420 -> png (and similar).
> It also "fixes" ticket #290.
> 
> Please review, Carl Eugen

> diff --git a/libavcodec/imgconvert.c b/libavcodec/imgconvert.c
> index bbeb5a5..e600d1c 100644
> --- a/libavcodec/imgconvert.c
> +++ b/libavcodec/imgconvert.c
> @@ -109,6 +109,7 @@ static const PixFmtInfo pix_fmt_info[PIX_FMT_NB] = {
>  
>      /* YUV formats with alpha plane */
>      [PIX_FMT_YUVA420P] = {
> +        .is_alpha = 1,
>          .color_type = FF_COLOR_YUV,
>      },

LGTM.

BTW this reminds me that such information should be moved to a more
generic position coupled with the pixdesc (possibly *not* in pixdesc
for the reasons stated many times), then I never managed to get
through it for a bunch of different reasons.

One possibility would be to move the imgconvert array to libavutil,
and design some functions for accessing that info (without directly
exposing it), e.g. av_get_pix_fmt_has_alpha() and
av_get_pix_fmt_colorspace(), such information could then be shared
amongst lavc, lavfi, lsws.

A better approach would require to keep pixel format component
disposition and chroma information separated (e.g. store colorspace
and color per component independently from pixdesc).

Thus for describing pixel format disposition + color info for YUVA420P
you would have:
pixdesc
colorspace -> e.g. AV_CS_YUV_ITU709
color_component_map -> { Y, U, V, A };

The simple alternative for getting the has_alpha information would be
to simply check on the number of components (odd number => has alpha)
as it is done in other parts of the code, not sure how robust is this
approach.
-- 
FFmpeg = Forgiving and Faithful Martial Philosophical Emblematic Governor