[FFmpeg-devel] [PATCH] Support HDR10+ metadata for HEVC.

Mohammad Izadi izadi at google.com
Fri Aug 7 19:51:52 EEST 2020


Any more comments?  Are you OK to merge?
Thanks,
Mohammad


On Thu, Jul 30, 2020 at 9:06 AM Vittorio Giovara <vittorio.giovara at gmail.com>
wrote:

> On Mon, Jul 27, 2020 at 11:44 PM Mohammad Izadi <
> izadi-at-google.com at ffmpeg.org> wrote:
>
> > It seems FATE is for the regression test. Here is a sample that you can
> use
> > and check:
> >
> >  https://www.dropbox.com/s/3ewr2t2lvv2cy8d/20200727_143643.mp4?dl=0
> >
> >
> Thanks I will check it out.
> Fate is indeed for regression testing, but also for continuous integration.
> If a portion of code has a fate sample available, it is automatically
> tested every time, and if there is a breaking change, people can act upon
> it and prevent it from happening.
> Vittorio
>
>
> > Thanks,
> > Mohammad
> >
> >
> > On Mon, Jul 27, 2020 at 7:53 AM Vittorio Giovara <
> > vittorio.giovara at gmail.com>
> > wrote:
> >
> > > On Fri, Jul 24, 2020 at 11:09 PM Mohammad Izadi <
> > > izadi-at-google.com at ffmpeg.org> wrote:
> > >
> > > > On Fri, Jul 24, 2020 at 9:30 AM Andreas Rheinhardt <
> > > > andreas.rheinhardt at gmail.com> wrote:
> > > >
> > > > > Mohammad Izadi:
> > > > > > From: Mohammad Izadi <moh.izadi at gmail.com>
> > > > > >
> > > > > > HDR10+ is dynamic metadata (A/341 Amendment - SMPTE2094-40) that
> > > needs
> > > > > to be decoded from ITU-T T.35 in HEVC bitstream. The HDR10+ is
> > > > transferred
> > > > > to side data packet to be used or passed through.
> > > > > > ---
> > > > > >  libavcodec/avpacket.c |   1 +
> > > > > >  libavcodec/decode.c   |   1 +
> > > > > >  libavcodec/hevc_sei.c |  39 ++++++---
> > > > > >  libavcodec/hevc_sei.h |   5 ++
> > > > > >  libavcodec/hevcdec.c  |  16 ++++
> > > > > >  libavcodec/internal.h |   9 +++
> > > > > >  libavcodec/packet.h   |   8 ++
> > > > > >  libavcodec/utils.c    | 180
> > > ++++++++++++++++++++++++++++++++++++++++++
> > > > > >  libavcodec/version.h  |   2 +-
> > > > > >  9 files changed, 248 insertions(+), 13 deletions(-)
> > > > > >
> > > > > > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> > > > > > index dce26cb31a..8307032335 <(830)%20703-2335>
> <(830)%20703-2335> <(830)%20703-2335>
> > > 100644
> > > > > > --- a/libavcodec/avpacket.c
> > > > > > +++ b/libavcodec/avpacket.c
> > > > > > @@ -394,6 +394,7 @@ const char *av_packet_side_data_name(enum
> > > > > AVPacketSideDataType type)
> > > > > >      case AV_PKT_DATA_CONTENT_LIGHT_LEVEL:        return "Content
> > > light
> > > > > level metadata";
> > > > > >      case AV_PKT_DATA_SPHERICAL:                  return
> "Spherical
> > > > > Mapping";
> > > > > >      case AV_PKT_DATA_A53_CC:                     return "A53
> > Closed
> > > > > Captions";
> > > > > > +    case AV_PKT_DATA_DYNAMIC_HDR_PLUS:           return "HDR10+
> > > > Dynamic
> > > > > Metadata (SMPTE 2094-40)";
> > > > > >      case AV_PKT_DATA_ENCRYPTION_INIT_INFO:       return
> > "Encryption
> > > > > initialization data";
> > > > > >      case AV_PKT_DATA_ENCRYPTION_INFO:            return
> > "Encryption
> > > > > info";
> > > > > >      case AV_PKT_DATA_AFD:                        return "Active
> > > Format
> > > > > Description data";
> > > > > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > > > > > index de9c079f9d..cd3286f7fb 100644
> > > > > > --- a/libavcodec/decode.c
> > > > > > +++ b/libavcodec/decode.c
> > > > > > @@ -1698,6 +1698,7 @@ int ff_decode_frame_props(AVCodecContext
> > > *avctx,
> > > > > AVFrame *frame)
> > > > > >          { AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
> > > > > AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
> > > > > >          { AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
> > > > > AV_FRAME_DATA_CONTENT_LIGHT_LEVEL },
> > > > > >          { AV_PKT_DATA_A53_CC,
> > >  AV_FRAME_DATA_A53_CC
> > > > > },
> > > > > > +        { AV_PKT_DATA_DYNAMIC_HDR_PLUS,
> > > > >  AV_FRAME_DATA_DYNAMIC_HDR_PLUS },
> > > > > >          { AV_PKT_DATA_ICC_PROFILE,
> > > > > AV_FRAME_DATA_ICC_PROFILE },
> > > > > >      };
> > > > > >
> > > > > > diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
> > > > > > index a4ec65dc1a..a490e752dd 100644
> > > > > > --- a/libavcodec/hevc_sei.c
> > > > > > +++ b/libavcodec/hevc_sei.c
> > > > > > @@ -25,6 +25,7 @@
> > > > > >  #include "golomb.h"
> > > > > >  #include "hevc_ps.h"
> > > > > >  #include "hevc_sei.h"
> > > > > > +#include "internal.h"
> > > > > >
> > > > > >  static int
> decode_nal_sei_decoded_picture_hash(HEVCSEIPictureHash
> > > *s,
> > > > > GetBitContext *gb)
> > > > > >  {
> > > > > > @@ -242,8 +243,8 @@ static int
> > > > > decode_nal_sei_user_data_unregistered(HEVCSEIUnregistered *s,
> GetBitC
> > > > > >  static int decode_nal_sei_user_data_registered_itu_t_t35(HEVCSEI
> > *s,
> > > > > GetBitContext *gb,
> > > > > >                                                           int
> size)
> > > > > >  {
> > > > > > -    uint32_t country_code;
> > > > > > -    uint32_t user_identifier;
> > > > > > +    uint8_t country_code;
> > > > > > +    uint16_t provider_code;
> > > > > >
> > > > > >      if (size < 7)
> > > > > >          return AVERROR(EINVAL);
> > > > > > @@ -255,18 +256,31 @@ static int
> > > > > decode_nal_sei_user_data_registered_itu_t_t35(HEVCSEI *s,
> GetBitConte
> > > > > >          size--;
> > > > > >      }
> > > > > >
> > > > > > -    skip_bits(gb, 8);
> > > > > > -    skip_bits(gb, 8);
> > > > > > -
> > > > > > -    user_identifier = get_bits_long(gb, 32);
> > > > > > -
> > > > > > -    switch (user_identifier) {
> > > > > > -        case MKBETAG('G', 'A', '9', '4'):
> > > > > > +    provider_code = get_bits(gb, 16);
> > > > > > +
> > > > > > +    const uint8_t usa_country_code = 0xB5;
> > > > > > +    const uint16_t smpte_provider_code = 0x003C;
> > > > > > +    if (country_code == usa_country_code &&
> > > > > > +        provider_code == smpte_provider_code) {
> > > > > > +        // A/341 Amendment – 2094-40
> > > > > > +        uint16_t provider_oriented_code = get_bits(gb, 16);
> > > > > > +        uint8_t application_identifier = get_bits(gb, 8);
> > > > > > +        const uint16_t smpte2094_40_provider_oriented_code =
> > 0x0001;
> > > > > > +        const uint16_t smpte2094_40_application_identifier =
> 0x04;
> > > > > > +        if (provider_oriented_code ==
> > > > > smpte2094_40_provider_oriented_code &&
> > > > > > +            application_identifier ==
> > > > > smpte2094_40_application_identifier) {
> > > > > > +            int err = ff_read_itu_t_t35_to_dynamic_hdr_plus(gb,
> > s->
> > > > > dynamic_hdr_plus.info);
> > > > > > +            if (err < 0 && s->dynamic_hdr_plus.info) {
> > > > > > +                av_buffer_unref(&s->dynamic_hdr_plus.info);
> > > > > > +            }
> > > > > > +            return err;
> > > > > > +        }
> > > > > > +    } else {
> > > > > > +        uint32_t  user_identifier = get_bits_long(gb, 32);
> > > > > > +        if(user_identifier == MKBETAG('G', 'A', '9', '4'))
> > > > > >              return
> > > > > decode_registered_user_data_closed_caption(&s->a53_caption, gb,
> > size);
> > > > > > -        default:
> > > > > > -            skip_bits_long(gb, size * 8);
> > > > > > -            break;
> > > > > >      }
> > > > > > +    skip_bits_long(gb, size * 8);
> > > > > >      return 0;
> > > > > >  }
> > > > > >
> > > > > > @@ -453,4 +467,5 @@ void ff_hevc_reset_sei(HEVCSEI *s)
> > > > > >          av_buffer_unref(&s->unregistered.buf_ref[i]);
> > > > > >      s->unregistered.nb_buf_ref = 0;
> > > > > >      av_freep(&s->unregistered.buf_ref);
> > > > > > +    av_buffer_unref(&s->dynamic_hdr_plus.info);
> > > > > >  }
> > > > > > diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
> > > > > > index 5ee7a4796d..e9e2d46ed4 100644
> > > > > > --- a/libavcodec/hevc_sei.h
> > > > > > +++ b/libavcodec/hevc_sei.h
> > > > > > @@ -104,6 +104,10 @@ typedef struct HEVCSEIMasteringDisplay {
> > > > > >      uint32_t min_luminance;
> > > > > >  } HEVCSEIMasteringDisplay;
> > > > > >
> > > > > > +typedef struct HEVCSEIDynamicHDRPlus {
> > > > > > +    AVBufferRef *info;
> > > > > > +} HEVCSEIDynamicHDRPlus;
> > > > > > +
> > > > > >  typedef struct HEVCSEIContentLight {
> > > > > >      int present;
> > > > > >      uint16_t max_content_light_level;
> > > > > > @@ -143,6 +147,7 @@ typedef struct HEVCSEI {
> > > > > >      HEVCSEIA53Caption a53_caption;
> > > > > >      HEVCSEIUnregistered unregistered;
> > > > > >      HEVCSEIMasteringDisplay mastering_display;
> > > > > > +    HEVCSEIDynamicHDRPlus dynamic_hdr_plus;
> > > > > >      HEVCSEIContentLight content_light;
> > > > > >      int active_seq_parameter_set_id;
> > > > > >      HEVCSEIAlternativeTransfer alternative_transfer;
> > > > > > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> > > > > > index b77df8d89f..cd794231e8 100644
> > > > > > --- a/libavcodec/hevcdec.c
> > > > > > +++ b/libavcodec/hevcdec.c
> > > > > > @@ -2849,6 +2849,16 @@ static int set_side_data(HEVCContext *s)
> > > > > >
> > > > > >          s->sei.timecode.num_clock_ts = 0;
> > > > > >      }
> > > > > > +    if (s->sei.dynamic_hdr_plus.info){
> > > > > > +      AVBufferRef *info_ref = av_buffer_ref(s->
> > > > > sei.dynamic_hdr_plus.info);
> > > > > > +      if (!info_ref)
> > > > > > +     return AVERROR(ENOMEM);
> > > > > > +
> > > > > > +      if(!av_frame_new_side_data_from_buf(out,
> > > > > AV_FRAME_DATA_DYNAMIC_HDR_PLUS, info_ref)){
> > > > > > +        av_buffer_unref(&info_ref);
> > > > > > +        return AVERROR(ENOMEM);
> > > > > > +      }
> > > > > > +    }
> > > > > >
> > > > > >      return 0;
> > > > > >  }
> > > > > > @@ -3530,6 +3540,12 @@ static int
> > > > > hevc_update_thread_context(AVCodecContext *dst,
> > > > > >          if (!s->sei.a53_caption.buf_ref)
> > > > > >              return AVERROR(ENOMEM);
> > > > > >      }
> > > > > > +    av_buffer_unref(&s->sei.dynamic_hdr_plus.info);
> > > > > > +    if (s0->sei.dynamic_hdr_plus.info) {
> > > > > > +        s->sei.dynamic_hdr_plus.info = av_buffer_ref(s0->
> > > > > sei.dynamic_hdr_plus.info);
> > > > > > +        if (!s->sei.dynamic_hdr_plus.info)
> > > > > > +            return AVERROR(ENOMEM);
> > > > > > +    }
> > > > > >
> > > > > >      s->sei.frame_packing        = s0->sei.frame_packing;
> > > > > >      s->sei.display_orientation  = s0->sei.display_orientation;
> > > > > > diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> > > > > > index 0a1c0a17ec..744ace534c 100644
> > > > > > --- a/libavcodec/internal.h
> > > > > > +++ b/libavcodec/internal.h
> > > > > > @@ -413,6 +413,15 @@ int ff_int_from_list_or_default(void *ctx,
> > const
> > > > > char * val_name, int val,
> > > > > >
> > > > > >  void ff_dvdsub_parse_palette(uint32_t *palette, const char *p);
> > > > > >
> > > > > > +/**
> > > > > > + * Reads and decode the user data registered ITU-T T.35 to
> > AVbuffer
> > > > > (AVDynamicHDRPlus).
> > > > > > + * @param gbc The bit content to be decoded.
> > > > > > + * @param output A buffer containing the decoded
> AVDynamicHDRPlus
> > > > > structure.
> > > > > > + *
> > > > > > + * @return 0 if succeed. Otherwise, returns the appropriate
> > AVERROR.
> > > > > > + */
> > > > > > +int ff_read_itu_t_t35_to_dynamic_hdr_plus(void *gbc, AVBufferRef
> > > > > *output);
> > > > >
> > > > > This can't work at all. output needs to be a AVBufferRef ** for it
> to
> > > > > return a AVBuffer Ref *.
> > > > >
> > > >
> > > > Done. Fixed in the next commit.
> > > >
> > >
> > > Hi, thanks for the patch, do you have a sample you could share to check
> > the
> > > correct behaviour?
> > > I don't remember if FATE supports testing for this kind of metadata,
> but
> > if
> > > so, it would be nice to add a test.
> > > --
> > > Vittorio
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > > To unsubscribe, visit link above, or email
> > > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
>
>
> --
> Vittorio
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


More information about the ffmpeg-devel mailing list