On Wed, Jul 10, 2019 at 4:23 AM Dennis Mungai <dmngaie at gmail.com> wrote:
>
> On Wed, Jul 10, 2019, 03:05 Aman Gupta <ffmpeg at tmm1.net> wrote:
>
> > On Wed, Jun 26, 2019 at 4:25 AM <lance.lmwang at gmail.com> wrote:
> >
> > > From: Limin Wang <lance.lmwang at gmail.com>
> > >
> > > Below is the testing ffmpeg command for the setting:
> > > ./ffmpeg -i input.ts -c:v hevc_videotoolbox -color_primaries bt2020
> > > -colorspace bt2020_ncl -color_trc smpte2084 smpte2048.ts
> > > ./ffmpeg -i input.ts -c:v hevc_videotoolbox -color_primaries bt2020
> > > -colorspace bt2020_ncl -color_trc linear linear.ts
> > > ./ffmpeg -i input.ts -c:v hevc_videotoolbox -color_primaries bt2020
> > > -colorspace bt2020_ncl -color_trc arib-std-b67 hlg.ts
> > >
> >
> > Patch looks reasonable to me. Will commit in a few days if no one else
> > comments.
> >
> > Aman
> >
> >
> > >
> > > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> > > ---
> > > configure | 6 ++++++
> > > libavcodec/videotoolboxenc.c | 16 ++++++++++++++++
> > > 2 files changed, 22 insertions(+)
> > >
> > > diff --git a/configure b/configure
> > > index 7cea9d4d73..0a5e940c0c 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -2260,6 +2260,9 @@ TOOLCHAIN_FEATURES="
> > > TYPES_LIST="
> > > kCMVideoCodecType_HEVC
> > > kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange
> > > + kCVImageBufferTransferFunction_SMPTE_ST_2084_PQ
> > > + kCVImageBufferTransferFunction_ITU_R_2100_HLG
> > > + kCVImageBufferTransferFunction_Linear
> > > socklen_t
> > > struct_addrinfo
> > > struct_group_source_req
> > > @@ -6044,6 +6047,9 @@ enabled videotoolbox && {
> > > check_lib coreservices CoreServices/CoreServices.h
> > > UTGetOSTypeFromString "-framework CoreServices"
> > > check_func_headers CoreMedia/CMFormatDescription.h
> > > kCMVideoCodecType_HEVC "-framework CoreMedia"
> > > check_func_headers CoreVideo/CVPixelBuffer.h
> > > kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange "-framework CoreVideo"
> > > + check_func_headers CoreVideo/CVImageBuffer.h
> > > kCVImageBufferTransferFunction_SMPTE_ST_2084_PQ "-framework CoreVideo"
> > > + check_func_headers CoreVideo/CVImageBuffer.h
> > > kCVImageBufferTransferFunction_ITU_R_2100_HLG "-framework CoreVideo"
> > > + check_func_headers CoreVideo/CVImageBuffer.h
> > > kCVImageBufferTransferFunction_Linear "-framework CoreVideo"
> > > }
> > >
> > > check_struct "sys/time.h sys/resource.h" "struct rusage" ru_maxrss
> > > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> > > index f8ccdea52d..0dc6eb4cf4 100644
> > > --- a/libavcodec/videotoolboxenc.c
> > > +++ b/libavcodec/videotoolboxenc.c
> > > @@ -915,6 +915,22 @@ static int get_cv_transfer_function(AVCodecContext
> > > *avctx,
> > > *transfer_fnc =
> > > kCVImageBufferTransferFunction_SMPTE_240M_1995;
> > > break;
> > >
> > > +#if HAVE_KCVIMAGEBUFFERTRANSFERFUNCTION_SMPTE_ST_2084_PQ
> > > + case AVCOL_TRC_SMPTE2084:
> > > + *transfer_fnc =
> > > kCVImageBufferTransferFunction_SMPTE_ST_2084_PQ;
> > > + break;
> > > +#endif
> > > +#if HAVE_KCVIMAGEBUFFERTRANSFERFUNCTION_LINEAR
> > > + case AVCOL_TRC_LINEAR:
> > > + *transfer_fnc = kCVImageBufferTransferFunction_Linear;
> > > + break;
> > > +#endif
> > > +#if HAVE_KCVIMAGEBUFFERTRANSFERFUNCTION_ITU_R_2100_HLG
> > > + case AVCOL_TRC_ARIB_STD_B67:
> > > + *transfer_fnc =
> > kCVImageBufferTransferFunction_ITU_R_2100_HLG;
> > > + break;
> > > +#endif
> > > +
> > > case AVCOL_TRC_GAMMA22:
> > > gamma = 2.2;
> > > *transfer_fnc = kCVImageBufferTransferFunction_UseGamma;
> > > --
> > > 2.21.0
> > >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > > To unsubscribe, visit link above, or email
> > > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >
>
> A while back, a similar patch was rejected for NVENC (from the same author)
> on the basis of such functionality not being suitably placed as an encoder
> feature.
>
> Looking back at other implementations, eg hevc_vaapi, the same is indeed
> allowed, where SEI metadata retention for HDR is present.
>
> To the OP: If the patch for NVENC were refactored in a manner similar to
> this, abstracting details such as master display info for HDR (something
> that was specifically flagged as not suitable as an encoder feature), it
> might pass the patch review and be merged.
>
> Are there specific limitations (as implemented) that block such a feature
> in NVENC? Perhaps the initial patch with mastering display controls for HDR
> was written that way to circumvent such a limitation.
>
I think you are confusing the type of patches here. This does nothing
but extend existing functionality to forward a few more values in
basically an enum that was already being set.
The other patch was about parsing user inputs and generating SEI data,
which should arguably be done in a central place instead of every
encoder individually.
- Hendrik