[FFmpeg-devel] [PATCH v9 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper

Mark Thompson sw at jkqxz.net
Wed Mar 27 23:19:49 EET 2019


On 27/03/2019 17:13, Vittorio Giovara wrote:
> On Tue, Mar 26, 2019 at 10:47 PM Jing Sun <jing.a.sun at intel.com> wrote:
> 
>> Signed-off-by: Zhengxu Huang <zhengxu.huang at intel.com>
>> Signed-off-by: Hassene Tmar <hassene.tmar at intel.com>
>> Signed-off-by: Jun Zhao <jun.zhao at intel.com>
>> Signed-off-by: Jing Sun <jing.a.sun at intel.com>
>> ---
>>  configure                |   4 +
>>  libavcodec/Makefile      |   1 +
>>  libavcodec/allcodecs.c   |   1 +
>>  libavcodec/libsvt_hevc.c | 502
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 508 insertions(+)
>>  create mode 100644 libavcodec/libsvt_hevc.c
>>
>> ...
>> +    if (svt_enc->hdr) {
>> +        svt_enc->vui_info = 1;
>> +        param->highDynamicRangeInput = svt_enc->hdr;
>> +    }
>>
> 
> Where is the warning that notifies the lack of color properties support?

Looking at what the highDynamicRangeInput field actually does <https://github.com/OpenVisualCloud/SVT-HEVC/blob/master/Source/Lib/Codec/EbResourceCoordinationProcess.c#L550-L566>, I don't think it makes sense for this external "hdr" option at exist at all.

>From the point of view of a user looking at the external options, they might expect that on setting this option some conversion takes place to actually create an HDR output.  In fact, that's not what it does - it just marks the output with some very specific colour properties, and any stream which doesn't actually have exactly those properties will then have incorrect metadata for display (possibly conflicting with container metadata, if the container has better support for colour properties than this encoder).

Perhaps to avoid confusion about what is actually happening the option could be removed and this check replaced with something like:

if (avctx->colorspace == AVCOL_SPC_BT2020_NCL &&
    avctx->color_primaries == AVCOL_PRI_BT2020 &&
    avctx->color_trc == AVCOL_TRC_SMPTE2084 &&
    avctx->color_range == AVCOL_RANGE_MPEG &&
    avctx->chroma_sample_location == AVCHROMA_LOC_TOPLEFT) {
    param->highDynamicRangeInput = 1;
} else {
    param->highDynamicRangeInput = 0;
    // Maybe also a warning here in some cases?
}

That would then do the right thing for all streams which actually have the given properties, while not forcing incorrect display of anything else.

- Mark


More information about the ffmpeg-devel mailing list