[FFmpeg-devel] [PATCH 2/4] vaapi_encode_h265: Insert mastering display colour colume if needed

Mark Thompson sw at jkqxz.net
Tue May 8 00:03:55 EEST 2018


On 04/05/18 09:54, Xiang, Haihao wrote:
> On Thu, 2018-05-03 at 22:43 +0100, Mark Thompson wrote:
>> On 03/05/18 04:07, Haihao Xiang wrote:
>>> '-sei xxx' is added to control SEI insertion, so far only mastering
>>> display colour colume is available for testing.
>>
>> Typo: "colume" (also in the commit title).
>>
> 
> Thanks for catching the typo, I will correct it in the new version of patch.
> 
>>> v2: use the mastering display parameters from
>>> AVMasteringDisplayMetadata, set SEI_MASTERING_DISPLAY to 8 to match
>>> the H.264 part and take VAAPIEncodeH265Context::sei_needed as a ORed
>>> value so that we needn't check the correspoding SEI message again when
>>> writting the header.
>>>
>>> Signed-off-by: Haihao Xiang <haihao.xiang at intel.com>
>>> ---
>>>  libavcodec/vaapi_encode_h265.c | 128
>>> ++++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 127 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
>>> index 5203c6871d..326fe4fe66 100644
>>> --- a/libavcodec/vaapi_encode_h265.c
>>> +++ b/libavcodec/vaapi_encode_h265.c
>>> @@ -24,15 +24,20 @@
>>>  #include "libavutil/avassert.h"
>>>  #include "libavutil/common.h"
>>>  #include "libavutil/opt.h"
>>> +#include "libavutil/mastering_display_metadata.h"
>>>  
>>>  #include "avcodec.h"
>>>  #include "cbs.h"
>>>  #include "cbs_h265.h"
>>>  #include "hevc.h"
>>> +#include "hevc_sei.h"
>>>  #include "internal.h"
>>>  #include "put_bits.h"
>>>  #include "vaapi_encode.h"
>>>  
>>> +enum {
>>> +    SEI_MASTERING_DISPLAY       = 0x08,
>>> +};
>>>  
>>>  typedef struct VAAPIEncodeH265Context {
>>>      unsigned int ctu_width;
>>> @@ -47,6 +52,9 @@ typedef struct VAAPIEncodeH265Context {
>>>      H265RawSPS sps;
>>>      H265RawPPS pps;
>>>      H265RawSlice slice;
>>> +    H265RawSEI sei;
>>> +
>>> +    H265RawSEIMasteringDiplayColourVolume mastering_display;
>>>  
>>>      int64_t last_idr_frame;
>>>      int pic_order_cnt;
>>> @@ -58,6 +66,7 @@ typedef struct VAAPIEncodeH265Context {
>>>      CodedBitstreamContext *cbc;
>>>      CodedBitstreamFragment current_access_unit;
>>>      int aud_needed;
>>> +    int sei_needed;
>>>  } VAAPIEncodeH265Context;
>>>  
>>>  typedef struct VAAPIEncodeH265Options {
>>> @@ -65,6 +74,7 @@ typedef struct VAAPIEncodeH265Options {
>>>      int aud;
>>>      int profile;
>>>      int level;
>>> +    int sei;
>>>  } VAAPIEncodeH265Options;
>>>  
>>>  
>>> @@ -175,6 +185,64 @@ fail:
>>>      return err;
>>>  }
>>>  
>>> +static int vaapi_encode_h265_write_extra_header(AVCodecContext *avctx,
>>> +                                                VAAPIEncodePicture *pic,
>>> +                                                int index, int *type,
>>> +                                                char *data, size_t
>>> *data_len)
>>> +{
>>> +    VAAPIEncodeContext      *ctx = avctx->priv_data;
>>> +    VAAPIEncodeH265Context *priv = ctx->priv_data;
>>> +    CodedBitstreamFragment   *au = &priv->current_access_unit;
>>> +    int err, i;
>>> +
>>> +    if (priv->sei_needed) {
>>> +        if (priv->aud_needed) {
>>> +            err = vaapi_encode_h265_add_nal(avctx, au, &priv->aud);
>>> +            if (err < 0)
>>> +                goto fail;
>>> +            priv->aud_needed = 0;
>>> +        }
>>> +
>>> +        memset(&priv->sei, 0, sizeof(priv->sei));
>>> +        priv->sei.nal_unit_header  = (H265RawNALUnitHeader) {
>>> +            .nal_unit_type         = HEVC_NAL_SEI_PREFIX,
>>> +            .nuh_layer_id          = 0,
>>> +            .nuh_temporal_id_plus1 = 1,
>>> +        };
>>> +
>>> +        i = 0;
>>> +
>>> +        if (priv->sei_needed & SEI_MASTERING_DISPLAY) {
>>> +            priv->sei.payload[i].payload_type =
>>> HEVC_SEI_TYPE_MASTERING_DISPLAY_INFO;
>>> +            priv->sei.payload[i].payload.mastering_display = priv-
>>>> mastering_display;
>>> +            ++i;
>>> +        }
>>> +
>>> +        priv->sei.payload_count = i;
>>> +        av_assert0(priv->sei.payload_count > 0);
>>> +
>>> +        err = vaapi_encode_h265_add_nal(avctx, au, &priv->sei);
>>> +        if (err < 0)
>>> +            goto fail;
>>> +        priv->sei_needed = 0;
>>> +
>>> +        err = vaapi_encode_h265_write_access_unit(avctx, data, data_len,
>>> au);
>>> +        if (err < 0)
>>> +            goto fail;
>>> +
>>> +        ff_cbs_fragment_uninit(priv->cbc, au);
>>> +
>>> +        *type = VAEncPackedHeaderRawData;
>>> +        return 0;
>>> +    } else {
>>> +        return AVERROR_EOF;
>>> +    }
>>> +
>>> +fail:
>>> +    ff_cbs_fragment_uninit(priv->cbc, au);
>>> +    return err;
>>> +}
>>> +
>>>  static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
>>>  {
>>>      VAAPIEncodeContext                *ctx = avctx->priv_data;
>>> @@ -587,6 +655,53 @@ static int
>>> vaapi_encode_h265_init_picture_params(AVCodecContext *avctx,
>>>          priv->aud_needed = 0;
>>>      }
>>>  
>>> +    priv->sei_needed = 0;
>>> +
>>> +    if ((opt->sei & SEI_MASTERING_DISPLAY) &&
>>> +        (pic->type == PICTURE_TYPE_I || pic->type == PICTURE_TYPE_IDR)) {
>>> +        AVFrameSideData *sd =
>>> +            av_frame_get_side_data(pic->input_image,
>>> +                                   AV_FRAME_DATA_MASTERING_DISPLAY_METADATA
>>> );
>>
>> Do you know when this side-data might be updated - can it arrive on a random
>> frame in the middle of the stream?  (And if so, what should we do about it?)
>>
> 
> According the doc below, I think it is reasonable to check this side-data for
> I/IDR frame only. I also checked some HDR streams and this side-data is added
> for I/IDR frame. 
> 
> "When a mastering display colour volume SEI message is present for any picture
> of a CLVS of a particular layer, a mastering display colour volume SEI message
> shall be present for the first picture of the CLVS. The mastering display colour
> volume SEI message persists for the current layer in decoding order from the
> current picture until the end of the CLVS. All mastering display colour volume
> SEI messages that apply to the same CLVS shall have the same content."

Right - that implies you want to write them for intra frames, but it doesn't say where they will be present on the input to the encoder.

For example, suppose we are doing a simple transcode of an H.265 input which has this metadata, and it has 60-frame GOP size.  So, there might be changes to the metadata on every 60th frame of the input to the encoder.  If we only look for the metadata on each frame which is going to be an intra frame on the output then we might miss some changes which are on frames which were intra on the input but we aren't encoding them as intra on the output.  Does that make sense?

>>> +
>>> +        if (sd) {
>>> +            AVMasteringDisplayMetadata *mdm =
>>> +                (AVMasteringDisplayMetadata *)sd->data;
>>> +
>>> +            if (mdm->has_primaries && mdm->has_luminance) {
>>> +                const int mapping[3] = {1, 2, 0};
>>> +                const int chroma_den = 50000;
>>> +                const int luma_den   = 10000;
>>> +
>>> +                for (i = 0; i < 3; i++) {
>>> +                    const int j = mapping[i];
>>> +                    priv->mastering_display.display_primaries_x[i] =
>>> +                        FFMIN(lrint(chroma_den *
>>> +                                    av_q2d(mdm->display_primaries[j][0])),
>>> +                              50000);
>>> +                    priv->mastering_display.display_primaries_y[i] =
>>> +                        FFMIN(lrint(chroma_den *
>>> +                                    av_q2d(mdm->display_primaries[j][1])),
>>> +                              50000);
>>> +                }
>>> +
>>> +                priv->mastering_display.white_point_x =
>>> +                    FFMIN(lrint(chroma_den * av_q2d(mdm->white_point[0])),
>>> +                          50000);
>>> +                priv->mastering_display.white_point_y =
>>> +                    FFMIN(lrint(chroma_den * av_q2d(mdm->white_point[1])),
>>> +                          50000);
>>> +
>>> +                priv->mastering_display.max_display_mastering_luminance =
>>> +                    lrint(luma_den * av_q2d(mdm->max_luminance));
>>> +                priv->mastering_display.min_display_mastering_luminance =
>>> +                    FFMIN(lrint(luma_den * av_q2d(mdm->min_luminance)),
>>> +                          priv-
>>>> mastering_display.max_display_mastering_luminance);
>>> +
>>> +                priv->sei_needed |= SEI_MASTERING_DISPLAY;
>>> +            }
>>> +        }
>>
>> There are has_primaries and has_luminance fields in AVMasteringDisplayMetadata
>> - do they need to be checked here?  If they don't matter then please add a
>> comment to that effect.
> 
> I think user may use has_primaries and has_luminance to indicate the side-data
> is valid or not.

Hmm.  Presumably we only get this structure if at least one of them is set.  Suppose one is valid and the other is not - what then?  Can we write some default values?  (Are what are the right default values?  Unlike with content-light-level there isn't a zero value to avoid the problem...)

>>> +    }
>>> +
>>>      vpic->decoded_curr_pic = (VAPictureHEVC) {
>>>          .picture_id    = pic->recon_surface,
>>>          .pic_order_cnt = priv->pic_order_cnt,
>>> @@ -895,6 +1010,8 @@ static const VAAPIEncodeType vaapi_encode_type_h265 = {
>>>  
>>>      .slice_header_type     = VAEncPackedHeaderHEVC_Slice,
>>>      .write_slice_header    = &vaapi_encode_h265_write_slice_header,
>>> +
>>> +    .write_extra_header    = &vaapi_encode_h265_write_extra_header,
>>>  };
>>>  
>>>  static av_cold int vaapi_encode_h265_init(AVCodecContext *avctx)
>>> @@ -943,7 +1060,8 @@ static av_cold int
>>> vaapi_encode_h265_init(AVCodecContext *avctx)
>>>  
>>>      ctx->va_packed_headers =
>>>          VA_ENC_PACKED_HEADER_SEQUENCE | // VPS, SPS and PPS.
>>> -        VA_ENC_PACKED_HEADER_SLICE;     // Slice headers.
>>> +        VA_ENC_PACKED_HEADER_SLICE    | // Slice headers.
>>> +        VA_ENC_PACKED_HEADER_MISC;      // SEI
>>>  
>>>      ctx->surface_width  = FFALIGN(avctx->width,  16);
>>>      ctx->surface_height = FFALIGN(avctx->height, 16);
>>> @@ -1003,6 +1121,14 @@ static const AVOption vaapi_encode_h265_options[] = {
>>>      { LEVEL("6.2", 186) },
>>>  #undef LEVEL
>>>  
>>> +    { "sei", "Set SEI to include",
>>> +      OFFSET(sei), AV_OPT_TYPE_FLAGS,
>>> +      { .i64 = SEI_MASTERING_DISPLAY },
>>> +      0, INT_MAX, FLAGS, "sei" },
>>> +    { "mastering_display", "Include mastering display colour volume",
>>> +      0, AV_OPT_TYPE_CONST, { .i64 = SEI_MASTERING_DISPLAY },
>>> +      INT_MIN, INT_MAX, FLAGS, "sei" },
>>> +
>>>      { NULL },
>>>  };
>>>  
>>>
>>
>> Ignoring the mastering display part, all the SEI logic looks good.
>>
>> Thanks,
>>
>> - Mark


More information about the ffmpeg-devel mailing list