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

Xiang, Haihao haihao.xiang at intel.com
Tue May 8 06:54:03 EEST 2018


On Mon, 2018-05-07 at 22:03 +0100, Mark Thompson wrote:
> 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_META
> > > > DATA
> > > > );
> > > 
> > > 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. 

Yes.

>  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?

Do you mean the input and output have different GOP size, so that maybe a frame
is intra on the output but it is not intra on the input? if so, how about
writing the latest metadata from intra frame on the input to intra frame on the
output?


> 
> > > > +
> > > > +        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...)

It seems the spec doesn't define the default values, so currently the metadata
is written when both of them are set, otherwise the metadata is ignored. 

> 
> > > > +    }
> > > > +
> > > >      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
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list