[FFmpeg-devel] [PATCH] avcodec/proresdec2: only set avctx->color* when values are specified

[Vittorio Giovara]

On Fri, Jan 18, 2019 at 6:43 AM Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:

> 2019-01-18 4:48 GMT+01:00, Neil Birkbeck <neil.birkbeck at gmail.com>:
> > On Thu, Jan 17, 2019 at 7:43 PM Neil Birkbeck <neil.birkbeck at gmail.com>
> > wrote:
> >
> >> This allows preservation of color values set from the container,
> >> while still letting the bitstream take precedent when its values
> >> are specified to some actual value (e.g., not *UNSPECIFIED).
> >>
> >> Signed-off-by: Neil Birkbeck <neil.birkbeck at gmail.com>
> >> ---
> >>  libavcodec/proresdec2.c | 9 ++++++---
> >>  1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
> >> index 6209c229c9..662ca7d48b 100644
> >> --- a/libavcodec/proresdec2.c
> >> +++ b/libavcodec/proresdec2.c
> >> @@ -262,9 +262,12 @@ static int decode_frame_header(ProresContext *ctx,
> >> const uint8_t *buf,
> >>          }
> >>      }
> >>
> >> -    avctx->color_primaries = buf[14];
> >> -    avctx->color_trc       = buf[15];
> >> -    avctx->colorspace      = buf[16];
> >> +    if (buf[14] != AVCOL_PRI_UNSPECIFIED)
> >> +        avctx->color_primaries = buf[14];
> >> +    if (buf[15] != AVCOL_TRC_UNSPECIFIED)
> >> +        avctx->color_trc       = buf[15];
> >> +    if (buf[16] != AVCOL_SPC_UNSPECIFIED)
> >> +        avctx->colorspace      = buf[16];
> >>      avctx->color_range     = AVCOL_RANGE_MPEG;
> >>
> >>      ptr   = buf + 20;
> >> --
> >> 2.20.1.321.g9e740568ce-goog
> >>
> >>
> > To add a bit more context. The patch is a fix for the case when prores
> > bitstream code points are explicitly set to unspecified and are
> overriding
> > what may have been in the container (unlike h264/h265 where such values
> can
> > behind the color_description flag, these fields always must be present in
> > the prores header).
>
> Isn't this even more of a reason to prefer the container value over
> the bitstream value?
>

Absolutely not
-- 
Vittorio