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

Neil Birkbeck neil.birkbeck at gmail.com
Sat Jan 19 06:39:29 EET 2019


On Fri, Jan 18, 2019 at 7:48 AM Vittorio Giovara <vittorio.giovara at gmail.com>
wrote:

> 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
>
I was initially going to check and see if preferring the container (when
set) was a valid alternative, since that was what the old behavior was. But
I couldn't find precedent for that elsewhere in the code (except when the
codec was just assuming some fixed constant not specified in the
bitstream). If a library user wants to prefer/detect container metadata, I
take it they have to avformat_open_input, inspect metadata before
find_stream_info, and use their application-specific logic to do add any
bitstream filters or adjust parameters of the filter graph?

For posterity, there are a couple more slightly relevant links I found when
looking for an authoritative reference on such inconsistencies in prores. I
only found [1] that confirms the tooling/workflows were inconsistent at
some point, and [2] which suggested that the colr atom took precedence over
headers on whatever viewer they were talking about (presumably quicktime,
on mac).

[1] https://github.com/bbc/qtff-parameter-editor
[2]
https://www.drastic.tv/support-59/supporttipstechnical/202-prores-colour-shifts-in-post-production
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avcodec-proresdec2-only-set-avctx-color-when-values-.patch
Type: application/x-patch
Size: 2420 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190118/e93750ed/attachment.bin>


More information about the ffmpeg-devel mailing list