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

Submitted by Haihao Xiang on May 16, 2018, 7:19 a.m.

Details

Message ID 20180516071945.28983-2-haihao.xiang@intel.com
State New
Headers show

Commit Message

Haihao Xiang May 16, 2018, 7:19 a.m.
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@intel.com>
---
 fftools/ffmpeg.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Mark Thompson May 21, 2018, 10:07 p.m.
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@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.

Thanks,

- Mark

Patch hide | download patch | download mbox

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];