On Wed, 2018-05-09 at 14:53 +0100, Mark Thompson wrote:
> On 09/05/18 05:48, Xiang, Haihao wrote:
> > On Tue, 2018-05-08 at 22:51 +0100, Mark Thompson wrote:
> > > On 08/05/18 04:54, Xiang, Haihao wrote:
> > > > 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.xi...@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,
> > > > > > > > + VAAPIEncodePict
> > > > > > > > ure
> > > > > > > > *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_DISP
> > > > > > > > LAY_
> > > > > > > > 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?
> > >
> > > Having thought about this a bit further, I think that would be the best
> > > answer
> > > here for now.
> > >
> > > To do better we need the ability to notice the change and start a new CLVS
> > > with a new IDR frame - I'm looking at this anyway along with the reference
> > > structure stuff, since there are other settings which want the same
> > > behaviour
> > > (SAR and colour properties carried by the AVFrame).
> >
> > I think we may do the above things in a new patch in future, e.g. check the
> > side-data of the AVFrame before vaapi_encode_get_next() and set a forced IDR
> > via
> > ctx->force_idr.
>
> Yes, that is what I was thinking. The side data can get checked at the same
> time as other possibly-changing parameters (like aspect ratio), and a new IDR
> frame / CLVS forced if it changes from what we are currently using.
>
> > It seems we can't know whether an input is intra via AVFrame.pict_type, do
> > you
> > have any idea?
>
> Correct, this isn't visible here. AVFrame.pict_type can be set by a
> libavcodec user to force key frames in particular places if it wants, but
> otherwise is unused on input to the encoder.
>
Ok, I will still use the original logic which should work when we force an IDR
frame for the matadata changes, I will add some comments as well in the new
version of patch.
Thanks
Haihao
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
