[FFmpeg-devel] [PATCH 2/3] avcodec/nvenc: add master display and light level sei for HDR10

[Dennis Mungai]

On Tue, May 28, 2019, 01:15 James Almer <jamrial at gmail.com> wrote:

> On 5/27/2019 7:13 PM, Timo Rothenpieler wrote:
> > On 27.05.2019 23:49, Lance Wang wrote:
> >> On Saturday, May 25, 2019, James Almer <jamrial at gmail.com> wrote:
> >>
> >>> On 5/22/2019 3:59 AM, lance.lmwang at gmail.com wrote:
> >>>> From: Limin Wang <lance.lmwang at gmail.com>
> >>>>
> >>>> The testing command for the HDR10 output with nvenc:
> >>>> $ ./ffmpeg_g -y -i 4K.mp4 -c:v hevc_nvenc -g 7 -color_primaries bt2020
> >>> -colorspace bt2020_ncl -color_trc smpte2084 -sei hdr10 \
> >>>>          -master_display "G(13250,34500)B(7500,3000)R(
> >>> 34000,16000)WP(15635,16450)L(10000000,50)" -max_cll "0, 0" test.ts
> >>>>
> >>>> Please notice it is preferable to use the frame sei side data than
> >>> master_display and max_cll paramters config
> >>>> ---
> >>>>   libavcodec/nvenc.c      | 129
> >>>> ++++++++++++++++++++++++++++++++++++++++
> >>>>   libavcodec/nvenc.h      |  18 ++++++
> >>>>   libavcodec/nvenc_hevc.c |  11 ++++
> >>>>   3 files changed, 158 insertions(+)
> >>>>
> >>>> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
> >>>> index 75dda6d689..3fd0eca4a5 100644
> >>>> --- a/libavcodec/nvenc.c
> >>>> +++ b/libavcodec/nvenc.c
> >>>> @@ -22,6 +22,9 @@
> >>>>   #include "config.h"
> >>>>
> >>>>   #include "nvenc.h"
> >>>> +#include "cbs_h265.h"
> >>>
> >>> This doesn't seem right. The encoder isn't using this framework at all.
> >>>
> >>> You're apparently including this only to get the
> >>> H265RawSEIMasteringDisplayColourVolume and
> >>> H265RawSEIContentLightLevelInfo structs, which you don't really need to
> >>> fill sei_data[i].payload
> >>>
> >>> OK,  I'll remove the dependent for the two structure and update the
> >>> patch.
> >>
> >>
> >
> > I'm still not convinced that all this belong in an encoder.
> > Can it really not be made into a filter that adds it as extra data to
> > frames? There got to be other frame consumers that use this kind of data.
>
> hevc_metadata (h265_metadata_bsf.c) already exists. It should be trivial
> to add support for this there.
>

Case in point: Some encoders, such as hevc_vaapi, present private codec
options for the retention of specific SEIs, such as HDR (and is on by
default), yet others, such as the QSV runtime, have no such options.

Wiring this into a bitstream filter (such as hevc_metadata), etc would
easily address such disparities without mucking around with convoluted
private encoder options.

>