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

[Mark Thompson]

On 24/05/18 07:29, Xiang, Haihao wrote:
> On Wed, 2018-05-23 at 03:27 +0200, Michael Niedermayer wrote:
>> 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
> 
> Do you mean the color in dec_ctx may be incorrect? If so, we should fix the
> decoder. 

No, I mean where the input properties in the decode context are correct but don't match the output because it gets changed during filtering.

For example:

ffmpeg -i bt709_input.mkv -vf colorspace=bt2020 bt2020_output.mkv

will have the output file marked as BT.709 after this patch, where previously it was "unspecified".  (Explicitly setting -color_primaries/-color_trc/-colorspace on the output works in both cases.)

- Mark