[FFmpeg-devel] [PATCH] [dnxhddec] Do not overwrite colorspace if the container has set it.

[Michael Niedermayer]

On Mon, Nov 27, 2017 at 06:43:32PM -0800, Steven Robertson wrote:
> It's... less wrong.
> 
> The VC-3 DNxHR update added a 'clv' field to an existing byte in the frame
> header to allow for the carriage of a limited set of color info (0 =
> 709/709, 1=2020/2020NCL, 2=2020/2020CL, 3=container specifies), but because
> that field is new-ish and assigns a meaning other than 'unspecified' to 0,
> we see files that have correct colorimetry in a 'colr' atom or equivalent
> (e.g. 2020/2020NCL with PQ transfer curve) but which do not set the 'clv'
> value, so it defaults to 0. For these media, the most correct choice is to
> trust the container over the file. Because DNxHD/DNxHR is getting more
> popular for delivering HDR, HDR requires setting a transfer function, and
> 'clv' doesn't have a way to specify the transfer function, DNxHR for HDR
> always needs to rely on the container for metadata, and this at least gets
> the issue out of the way for those cases without changing existing behavior
> for containers that don't specify a color system.
> 
> Additionally, because it's set in the frame header, we'd have to probe a
> frame and guard against frame changes or even reconfigure things after a
> frame change, and since I'm not an experienced ffmpeg dev I opted for the
> minimal fix to make 2020NCL for HDR files work properly.

Something like this should be in the commit message

thx

> 
> 
> On Mon, Nov 27, 2017 at 4:35 PM, Michael Niedermayer <michael at niedermayer.cc
> > wrote:
> 
> > On Mon, Nov 27, 2017 at 12:36:48AM -0800, Steven Robertson wrote:
> > > Signed-off-by: Steven Robertson <steven at strobe.cc>
> > > ---
> > >  libavcodec/dnxhddec.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/dnxhddec.c b/libavcodec/dnxhddec.c
> > > index f46e41a456..6f8c716412 100644
> > > --- a/libavcodec/dnxhddec.c
> > > +++ b/libavcodec/dnxhddec.c
> > > @@ -93,7 +93,9 @@ static av_cold int dnxhd_decode_init(AVCodecContext
> > *avctx)
> > >
> > >      ctx->avctx = avctx;
> > >      ctx->cid = -1;
> > > -    avctx->colorspace = AVCOL_SPC_BT709;
> > > +    if (avctx->colorspace == AVCOL_SPC_UNSPECIFIED) {
> > > +      avctx->colorspace = AVCOL_SPC_BT709;
> > > +    }
> >
> > why is this sometimes but not always correct ?
> >
> > can one and the same dnxhd stream be 2 different colorspace depending
> > on container or is the decoder implementation incomplete in relation
> > to setting the colorspace ?
> >
> > [...]
> > --
> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > When the tyrant has disposed of foreign enemies by conquest or treaty, and
> > there is nothing more to fear from them, then he is always stirring up
> > some war or other, in order that the people may require a leader. -- Plato
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> >
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"You are 36 times more likely to die in a bathtub than at the hands of a
terrorist. Also, you are 2.5 times more likely to become a president and
2 times more likely to become an astronaut, than to die in a terrorist
attack." -- Thoughty2

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171128/0fe29861/attachment.sig>