Hi, On Fri, Apr 23, 2021 at 8:58 AM Mohammad Izadi <izadi-at-google....@ffmpeg.org> wrote: > > HDR10+ metadata is stored in the bit stream for HEVC. The story is different > for VP9 and cannot store the metadata in the bit stream. HDR10+ should be > passed to packet side data an stored in the container (mkv) for VP9. > > This CL is taking HDR10+ from AVFrame side data in libvpxenc and is passing > it to the AVPacket side data. > --- > libavcodec/avpacket.c | 1 + > libavcodec/decode.c | 1 + > libavcodec/libvpxenc.c | 71 ++++++++++++++++++++++++++++++++++++++++++ > libavcodec/packet.h | 10 +++++- > 4 files changed, 82 insertions(+), 1 deletion(-) >
Just some quick cosmetics, I didn't take a close look at the implementation. > [...] > +static int copy_hdr10_plus_to_pkt(void *list, AVPacket *pkt) > +{ > + struct FrameHDR10PlusList **p = list; > + struct FrameHDR10PlusList *head = *p; > + > + if(head && pkt && head->hdr10_plus && head->pts == pkt->pts) { Add whitespace after 'if' and elsewhere before the '{'. You can use tools/patcheck as a guide to the formatting [1]. > [...] > > @@ -1245,6 +1300,12 @@ FF_ENABLE_DEPRECATION_WARNINGS > AV_WB64(side_data, 1); > memcpy(side_data + 8, cx_frame->buf_alpha, cx_frame->sz_alpha); > } > + if(cx_frame->frame_number != -1) { // Not invisible frame VP9 won't emit frames like this, but this is a shared path with VP8. Can we get this metadata (unnecessarily) when doing a 8-bit encode? > + VPxContext *ctx = avctx->priv_data; > + int err = copy_hdr10_plus_to_pkt(&ctx->hdr10_plus_list, pkt); > + if (err < 0) > + return err; > + } > } else { > return ret; > } > @@ -1579,6 +1640,7 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket > *pkt, > const struct vpx_codec_enc_cfg *enccfg = ctx->encoder.config.enc; > vpx_svc_layer_id_t layer_id; > int layer_id_valid = 0; > + AVFrameSideData *hdr10_plus_metadata; > > if (frame) { > const AVFrameSideData *sd = av_frame_get_side_data(frame, > AV_FRAME_DATA_REGIONS_OF_INTEREST); > @@ -1655,6 +1717,15 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket > *pkt, > vp9_encode_set_roi(avctx, frame->width, frame->height, sd); > } > } > + > + // Add HDR10+ metadata to queue. > + hdr10_plus_metadata = av_frame_get_side_data(frame, > AV_FRAME_DATA_DYNAMIC_HDR_PLUS); > + if(hdr10_plus_metadata){ > + struct FrameHDR10PlusList *data = av_malloc(sizeof(struct > FrameHDR10PlusList)); This allocation should be checked and prefer sizeof(*data). > + data->pts = frame->pts; > + data->hdr10_plus = av_buffer_ref(hdr10_plus_metadata->buf); > + add_hdr10_plus(&ctx->hdr10_plus_list, data); > + } > } > > // this is for encoding with preset temporal layering patterns defined in > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > index ca18ae631f..89e683b357 100644 > --- a/libavcodec/packet.h > +++ b/libavcodec/packet.h > @@ -298,7 +298,15 @@ enum AVPacketSideDataType { > * If its value becomes huge, some code using it > * needs to be updated as it assumes it to be smaller than other limits. > */ > - AV_PKT_DATA_NB > + AV_PKT_DATA_NB, This should be the final entry in the enum, see the comment above. > + > + /** > + * HDR10+ dynamic metadata associated with a video frame. The metadata > is in > + * the form of the AVDynamicHDRPlus struct and contains > + * information for color volume transform - application 4 of > + * SPMTE 2094-40:2016 standard. > + */ > + AV_PKT_DATA_DYNAMIC_HDR10_PLUS > }; > > #define AV_PKT_DATA_QUALITY_FACTOR AV_PKT_DATA_QUALITY_STATS //DEPRECATED [1] https://ffmpeg.org/developer.html#Coding-Rules-1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".