[FFmpeg-devel] [PATCH] Support HDR10+ metadata for HEVC.
Mohammad Izadi
izadi at google.com
Wed Aug 12 19:40:24 EEST 2020
Vittorio,
What is the next step for me?
Thanks,
Mohammad
On Fri, Aug 7, 2020 at 9:51 AM Mohammad Izadi <izadi at google.com> wrote:
> 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