[FFmpeg-devel] [PATCH] avcodec/dpxenc: support colour metadata in DPX encoder, fixes ticket #6023

[Vittorio Giovara]

On Fri, Feb 3, 2017 at 2:10 PM, Kieran O Leary <kieran.o.leary at gmail.com> wrote:
> Hi Vittorio!
>
> thanks for getting back to me.
>
> On Fri, Feb 3, 2017 at 12:57 PM, Vittorio Giovara
> <vittorio.giovara at gmail.com> wrote:
>>
>>
>>
>> Hey Kieran,
>> I think the code looks fine. I am just wondering if we should also
>> offer the possibility to set these flags from the standard context
>> options (-color_trc and others). I'm aware that not all values match
>> or are valid but maybe a small conversion table or extending the main
>> table could be a viable approach. Similarly this could be done for the
>> decoder so that color properties are not lost during a dpx->dpx
>> conversion maybe.
>
>
> That seems to be the general consensus from the replies from James Almer and
> Carl Eugen and it's what i should push towards.
> I added the new values locally to pixfmt.h. I'm thinking that these could be
> called in a similar way to the EXR decoder?
> https://github.com/FFmpeg/FFmpeg/blob/8a1759ad46f05375c957f33049b4592befbcb224/libavcodec/exr.c#L1840

Not sure to which changes you mean, all the values listed by that
commit are already supported by the current
AVColorTransferCharacteristic implementation. Incidentally, I believe
that the codec you point to is a perfect examples of something
libavcodec should not do, color conversion in a decoder: in my opinion
this a task that should be reserved for something external such as
lavfi or ideally lsws.

> In terms of translation tables, could you point me to some simlar code that
> could serve as a starting point for me? The nearest that made sense to me
> seems to be these values in vf_colorpsace.c
> https://github.com/FFmpeg/FFmpeg/blob/master/libavfilter/vf_colorspace.c#L97
> ?

I didn't explain myself well enough, I was mainly suggesting that
rather than having a private option used to tag the final output, you
could *also* read the standard command line option in avctx->color_trc
and use it to tag the output. Since you can't reuse the value because
dpx seem to use a different table, you should translate the value from
AVColorTransferCharacteristic to whatever dpx accepts. In pseudocode

if (priv_trc_opt = "") {
   if (avctx->color_trc == AVCOL_BT709)
      buf[801] = DPX_BT709
   else if (avctx->color_trc == AVCOL_BT601)
      buf[801] = DPX_BT601
   ...
} else
   buf[801] = priv_trc_opt

-- 
Vittorio