[FFmpeg-devel] [PATCH 2/2] ffmpeg: Use the colour properties from the input stream when doing transcode

[Michael Niedermayer]

On Mon, May 21, 2018 at 11:07:34PM +0100, Mark Thompson wrote:
> On 16/05/18 08:19, Haihao Xiang wrote:
> > In lavc/hevec_vaapi, colour properties in AVCodecContext are needed to
> > write the sequence header
> > 
> > Tested by the command below:
> > ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128
> > -hwaccel_output_format vaapi -i input-with-hdr.mkv -c:v hevc_vaapi
> > -profile:v main10 output.h265
> > 
> > Signed-off-by: Haihao Xiang <haihao.xiang at intel.com>
> > ---
> >  fftools/ffmpeg.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > index 5a19a09d9a..80b5441f8f 100644
> > --- a/fftools/ffmpeg.c
> > +++ b/fftools/ffmpeg.c
> > @@ -3298,6 +3298,10 @@ static int init_output_stream_encode(OutputStream *ost)
> >          dec_ctx = ist->dec_ctx;
> >  
> >          enc_ctx->chroma_sample_location = dec_ctx->chroma_sample_location;
> > +        enc_ctx->color_range            = dec_ctx->color_range;
> > +        enc_ctx->color_primaries        = dec_ctx->color_primaries;
> > +        enc_ctx->color_trc              = dec_ctx->color_trc;
> > +        enc_ctx->colorspace             = dec_ctx->colorspace;
> >      } else {
> >          for (j = 0; j < oc->nb_streams; j++) {
> >              AVStream *st = oc->streams[j];
> > 
> 
> There are outstanding patches passing color_range through the filter chain (see <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-May/229248.html>), and that would be the right solution for the rest of these properties as well, but it would require significantly more work.
> 
> I think hacking it like this is ok for now?  It's not worse than before since a change during filtering wasn't visible anyway, and the manual options do still work to override it.  I think the commit message could be a little clearer about the problem (really, that this colour information doesn't pass through filtering) and its limitations (requires a single input matched to the output; will write the wrong thing if filtering changes anything) though, or maybe that could be explained in a comment.
> 
> Does anyone else have any thoughts on this?  If there are no objections then I would apply it updated with a clearer explanation.

I think the key question is
Does this change "Unknown" to incorrect values
if so it is IMHO not a good idea as we would create broken files where we did
previously just create files that lacked some metadata


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180523/c66a7d0b/attachment.sig>