Vittorio, What is the next step for me?
Thanks, Mohammad On Fri, Aug 7, 2020 at 9:51 AM Mohammad Izadi <iz...@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.giov...@gmail.com> wrote: > >> On Mon, Jul 27, 2020 at 11:44 PM Mohammad Izadi < >> izadi-at-google....@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.giov...@gmail.com> >> > wrote: >> > >> > > On Fri, Jul 24, 2020 at 11:09 PM Mohammad Izadi < >> > > izadi-at-google....@ffmpeg.org> wrote: >> > > >> > > > On Fri, Jul 24, 2020 at 9:30 AM Andreas Rheinhardt < >> > > > andreas.rheinha...@gmail.com> wrote: >> > > > >> > > > > Mohammad Izadi: >> > > > > > From: Mohammad Izadi <moh.iz...@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@ffmpeg.org >> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > >> > > To unsubscribe, visit link above, or email >> > > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". >> > _______________________________________________ >> > 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". >> >> >> >> -- >> Vittorio >> _______________________________________________ >> 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". > > _______________________________________________ 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".
