diff mbox series

[FFmpeg-devel,v7,1/2] avcodec/avutil: move dynamic HDR10+ metadata parsing to libavutil

Message ID f65d4e6e-5222-36b0-9ffc-ac02edd00f3b@vimeo.com
State New
Headers show
Series [FFmpeg-devel,v7,1/2] avcodec/avutil: move dynamic HDR10+ metadata parsing to libavutil | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Raphaël Zumer March 14, 2023, 11:04 p.m. UTC
Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
---
 libavcodec/Makefile              |   6 +-
 libavcodec/av1dec.c              |   6 +-
 libavcodec/dynamic_hdr10_plus.c  | 198 -------------------------------
 libavcodec/dynamic_hdr10_plus.h  |  35 ------
 libavcodec/h2645_sei.c           |   6 +-
 libavcodec/libdav1d.c            |   6 +-
 libavutil/hdr_dynamic_metadata.c | 185 +++++++++++++++++++++++++++++
 libavutil/hdr_dynamic_metadata.h |  13 ++
 8 files changed, 210 insertions(+), 245 deletions(-)
 delete mode 100644 libavcodec/dynamic_hdr10_plus.c
 delete mode 100644 libavcodec/dynamic_hdr10_plus.h

Comments

Andreas Rheinhardt March 16, 2023, 8:19 p.m. UTC | #1
Raphaël Zumer:
> Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
> ---
>  libavcodec/Makefile              |   6 +-
>  libavcodec/av1dec.c              |   6 +-
>  libavcodec/dynamic_hdr10_plus.c  | 198 -------------------------------
>  libavcodec/dynamic_hdr10_plus.h  |  35 ------
>  libavcodec/h2645_sei.c           |   6 +-
>  libavcodec/libdav1d.c            |   6 +-
>  libavutil/hdr_dynamic_metadata.c | 185 +++++++++++++++++++++++++++++
>  libavutil/hdr_dynamic_metadata.h |  13 ++
>  8 files changed, 210 insertions(+), 245 deletions(-)
>  delete mode 100644 libavcodec/dynamic_hdr10_plus.c
>  delete mode 100644 libavcodec/dynamic_hdr10_plus.h
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index abae4909d2..408ecd1e31 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -104,7 +104,7 @@ OBJS-$(CONFIG_H264_SEI)                += h264_sei.o h2645_sei.o
>  OBJS-$(CONFIG_HEVCPARSE)               += hevc_parse.o hevc_ps.o hevc_data.o \
>                                            h2645data.o h2645_parse.o h2645_vui.o
>  OBJS-$(CONFIG_HEVC_SEI)                += hevc_sei.o h2645_sei.o \
> -                                          dynamic_hdr10_plus.o dynamic_hdr_vivid.o
> +                                          dynamic_hdr_vivid.o
>  OBJS-$(CONFIG_HPELDSP)                 += hpeldsp.o
>  OBJS-$(CONFIG_HUFFMAN)                 += huffman.o
>  OBJS-$(CONFIG_HUFFYUVDSP)              += huffyuvdsp.o
> @@ -250,7 +250,7 @@ OBJS-$(CONFIG_ATRAC3PAL_DECODER)       += atrac3plusdec.o atrac3plus.o \
>  OBJS-$(CONFIG_ATRAC9_DECODER)          += atrac9dec.o
>  OBJS-$(CONFIG_AURA_DECODER)            += cyuv.o
>  OBJS-$(CONFIG_AURA2_DECODER)           += aura.o
> -OBJS-$(CONFIG_AV1_DECODER)             += av1dec.o dynamic_hdr10_plus.o
> +OBJS-$(CONFIG_AV1_DECODER)             += av1dec.o
>  OBJS-$(CONFIG_AV1_CUVID_DECODER)       += cuviddec.o
>  OBJS-$(CONFIG_AV1_MEDIACODEC_DECODER)  += mediacodecdec.o
>  OBJS-$(CONFIG_AV1_NVENC_ENCODER)       += nvenc_av1.o nvenc.o
> @@ -1082,7 +1082,7 @@ OBJS-$(CONFIG_LIBARIBB24_DECODER)         += libaribb24.o ass.o
>  OBJS-$(CONFIG_LIBCELT_DECODER)            += libcelt_dec.o
>  OBJS-$(CONFIG_LIBCODEC2_DECODER)          += libcodec2.o
>  OBJS-$(CONFIG_LIBCODEC2_ENCODER)          += libcodec2.o
> -OBJS-$(CONFIG_LIBDAV1D_DECODER)           += libdav1d.o dynamic_hdr10_plus.o
> +OBJS-$(CONFIG_LIBDAV1D_DECODER)           += libdav1d.o
>  OBJS-$(CONFIG_LIBDAVS2_DECODER)           += libdavs2.o
>  OBJS-$(CONFIG_LIBFDK_AAC_DECODER)         += libfdk-aacdec.o
>  OBJS-$(CONFIG_LIBFDK_AAC_ENCODER)         += libfdk-aacenc.o
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index a80e37e33f..df393fe3d0 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -20,6 +20,7 @@
>  
>  #include "config_components.h"
>  
> +#include "libavutil/hdr_dynamic_metadata.h"
>  #include "libavutil/film_grain_params.h"
>  #include "libavutil/mastering_display_metadata.h"
>  #include "libavutil/pixdesc.h"
> @@ -30,7 +31,6 @@
>  #include "bytestream.h"
>  #include "codec_internal.h"
>  #include "decode.h"
> -#include "dynamic_hdr10_plus.h"
>  #include "hwconfig.h"
>  #include "profiles.h"
>  #include "thread.h"
> @@ -925,8 +925,8 @@ static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame,
>          if (!hdrplus)
>              return AVERROR(ENOMEM);
>  
> -        ret = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer,
> -                                                       bytestream2_get_bytes_left(&gb));
> +        ret = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer,
> +                                           bytestream2_get_bytes_left(&gb));
>          if (ret < 0)
>              return ret;
>          break;
> diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c
> deleted file mode 100644
> index 34a44aac65..0000000000
> --- a/libavcodec/dynamic_hdr10_plus.c
> +++ /dev/null
> @@ -1,198 +0,0 @@
> -/*
> - * This file is part of FFmpeg.
> - *
> - * FFmpeg is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU Lesser General Public
> - * License as published by the Free Software Foundation; either
> - * version 2.1 of the License, or (at your option) any later version.
> - *
> - * FFmpeg is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * Lesser General Public License for more details.
> - *
> - * You should have received a copy of the GNU Lesser General Public
> - * License along with FFmpeg; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> - */
> -
> -#include "dynamic_hdr10_plus.h"
> -#include "get_bits.h"
> -
> -static const int64_t luminance_den = 1;
> -static const int32_t peak_luminance_den = 15;
> -static const int64_t rgb_den = 100000;
> -static const int32_t fraction_pixel_den = 1000;
> -static const int32_t knee_point_den = 4095;
> -static const int32_t bezier_anchor_den = 1023;
> -static const int32_t saturation_weight_den = 8;
> -
> -int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
> -                                             int size)
> -{
> -    GetBitContext gbc, *gb = &gbc;
> -    int ret;
> -
> -    if (!s)
> -        return AVERROR(ENOMEM);
> -
> -    ret = init_get_bits8(gb, data, size);
> -    if (ret < 0)
> -        return ret;
> -
> -    if (get_bits_left(gb) < 10)
> -        return AVERROR_INVALIDDATA;
> -
> -    s->application_version = get_bits(gb, 8);
> -    s->num_windows = get_bits(gb, 2);
> -
> -    if (s->num_windows < 1 || s->num_windows > 3) {
> -        return AVERROR_INVALIDDATA;
> -    }
> -
> -    if (get_bits_left(gb) < ((19 * 8 + 1) * (s->num_windows - 1)))
> -        return AVERROR_INVALIDDATA;
> -
> -    for (int w = 1; w < s->num_windows; w++) {
> -        // The corners are set to absolute coordinates here. They should be
> -        // converted to the relative coordinates (in [0, 1]) in the decoder.
> -        AVHDRPlusColorTransformParams *params = &s->params[w];
> -        params->window_upper_left_corner_x =
> -            (AVRational){get_bits(gb, 16), 1};
> -        params->window_upper_left_corner_y =
> -            (AVRational){get_bits(gb, 16), 1};
> -        params->window_lower_right_corner_x =
> -            (AVRational){get_bits(gb, 16), 1};
> -        params->window_lower_right_corner_y =
> -            (AVRational){get_bits(gb, 16), 1};
> -
> -        params->center_of_ellipse_x = get_bits(gb, 16);
> -        params->center_of_ellipse_y = get_bits(gb, 16);
> -        params->rotation_angle = get_bits(gb, 8);
> -        params->semimajor_axis_internal_ellipse = get_bits(gb, 16);
> -        params->semimajor_axis_external_ellipse = get_bits(gb, 16);
> -        params->semiminor_axis_external_ellipse = get_bits(gb, 16);
> -        params->overlap_process_option = get_bits1(gb);
> -    }
> -
> -    if (get_bits_left(gb) < 28)
> -        return AVERROR_INVALIDDATA;
> -
> -    s->targeted_system_display_maximum_luminance =
> -        (AVRational){get_bits_long(gb, 27), luminance_den};
> -    s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
> -
> -    if (s->targeted_system_display_actual_peak_luminance_flag) {
> -        int rows, cols;
> -        if (get_bits_left(gb) < 10)
> -            return AVERROR_INVALIDDATA;
> -        rows = get_bits(gb, 5);
> -        cols = get_bits(gb, 5);
> -        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
> -            return AVERROR_INVALIDDATA;
> -        }
> -        s->num_rows_targeted_system_display_actual_peak_luminance = rows;
> -        s->num_cols_targeted_system_display_actual_peak_luminance = cols;
> -
> -        if (get_bits_left(gb) < (rows * cols * 4))
> -            return AVERROR_INVALIDDATA;
> -
> -        for (int i = 0; i < rows; i++) {
> -            for (int j = 0; j < cols; j++) {
> -                s->targeted_system_display_actual_peak_luminance[i][j] =
> -                    (AVRational){get_bits(gb, 4), peak_luminance_den};
> -            }
> -        }
> -    }
> -    for (int w = 0; w < s->num_windows; w++) {
> -        AVHDRPlusColorTransformParams *params = &s->params[w];
> -        if (get_bits_left(gb) < (3 * 17 + 17 + 4))
> -            return AVERROR_INVALIDDATA;
> -
> -        for (int i = 0; i < 3; i++) {
> -            params->maxscl[i] =
> -                (AVRational){get_bits(gb, 17), rgb_den};
> -        }
> -        params->average_maxrgb =
> -            (AVRational){get_bits(gb, 17), rgb_den};
> -        params->num_distribution_maxrgb_percentiles = get_bits(gb, 4);
> -
> -        if (get_bits_left(gb) <
> -            (params->num_distribution_maxrgb_percentiles * 24))
> -            return AVERROR_INVALIDDATA;
> -
> -        for (int i = 0; i < params->num_distribution_maxrgb_percentiles; i++) {
> -            params->distribution_maxrgb[i].percentage = get_bits(gb, 7);
> -            params->distribution_maxrgb[i].percentile =
> -                (AVRational){get_bits(gb, 17), rgb_den};
> -        }
> -
> -        if (get_bits_left(gb) < 10)
> -            return AVERROR_INVALIDDATA;
> -
> -        params->fraction_bright_pixels = (AVRational){get_bits(gb, 10), fraction_pixel_den};
> -    }
> -    if (get_bits_left(gb) < 1)
> -        return AVERROR_INVALIDDATA;
> -    s->mastering_display_actual_peak_luminance_flag = get_bits1(gb);
> -    if (s->mastering_display_actual_peak_luminance_flag) {
> -        int rows, cols;
> -        if (get_bits_left(gb) < 10)
> -            return AVERROR_INVALIDDATA;
> -        rows = get_bits(gb, 5);
> -        cols = get_bits(gb, 5);
> -        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
> -            return AVERROR_INVALIDDATA;
> -        }
> -        s->num_rows_mastering_display_actual_peak_luminance = rows;
> -        s->num_cols_mastering_display_actual_peak_luminance = cols;
> -
> -        if (get_bits_left(gb) < (rows * cols * 4))
> -            return AVERROR_INVALIDDATA;
> -
> -        for (int i = 0; i < rows; i++) {
> -            for (int j = 0; j < cols; j++) {
> -                s->mastering_display_actual_peak_luminance[i][j] =
> -                    (AVRational){get_bits(gb, 4), peak_luminance_den};
> -            }
> -        }
> -    }
> -
> -    for (int w = 0; w < s->num_windows; w++) {
> -        AVHDRPlusColorTransformParams *params = &s->params[w];
> -        if (get_bits_left(gb) < 1)
> -            return AVERROR_INVALIDDATA;
> -
> -        params->tone_mapping_flag = get_bits1(gb);
> -        if (params->tone_mapping_flag) {
> -            if (get_bits_left(gb) < 28)
> -                return AVERROR_INVALIDDATA;
> -
> -            params->knee_point_x =
> -                (AVRational){get_bits(gb, 12), knee_point_den};
> -            params->knee_point_y =
> -                (AVRational){get_bits(gb, 12), knee_point_den};
> -            params->num_bezier_curve_anchors = get_bits(gb, 4);
> -
> -            if (get_bits_left(gb) < (params->num_bezier_curve_anchors * 10))
> -                return AVERROR_INVALIDDATA;
> -
> -            for (int i = 0; i < params->num_bezier_curve_anchors; i++) {
> -                params->bezier_curve_anchors[i] =
> -                    (AVRational){get_bits(gb, 10), bezier_anchor_den};
> -            }
> -        }
> -
> -        if (get_bits_left(gb) < 1)
> -            return AVERROR_INVALIDDATA;
> -        params->color_saturation_mapping_flag = get_bits1(gb);
> -        if (params->color_saturation_mapping_flag) {
> -            if (get_bits_left(gb) < 6)
> -                return AVERROR_INVALIDDATA;
> -            params->color_saturation_weight =
> -                (AVRational){get_bits(gb, 6), saturation_weight_den};
> -        }
> -    }
> -
> -    return 0;
> -}
> diff --git a/libavcodec/dynamic_hdr10_plus.h b/libavcodec/dynamic_hdr10_plus.h
> deleted file mode 100644
> index cd7acf0432..0000000000
> --- a/libavcodec/dynamic_hdr10_plus.h
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -/*
> - * This file is part of FFmpeg.
> - *
> - * FFmpeg is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU Lesser General Public
> - * License as published by the Free Software Foundation; either
> - * version 2.1 of the License, or (at your option) any later version.
> - *
> - * FFmpeg is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * Lesser General Public License for more details.
> - *
> - * You should have received a copy of the GNU Lesser General Public
> - * License along with FFmpeg; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> - */
> -
> -#ifndef AVCODEC_DYNAMIC_HDR10_PLUS_H
> -#define AVCODEC_DYNAMIC_HDR10_PLUS_H
> -
> -#include "libavutil/hdr_dynamic_metadata.h"
> -
> -/**
> - * Parse the user data registered ITU-T T.35 to AVbuffer (AVDynamicHDRPlus).
> - * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
> - * @param data The byte array containing the raw ITU-T T.35 data.
> - * @param size Size of the data array in bytes.
> - *
> - * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
> - */
> -int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
> -                                             int size);
> -
> -#endif /* AVCODEC_DYNAMIC_HDR10_PLUS_H */
> diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
> index 6e4a9a1af2..63ab711bc9 100644
> --- a/libavcodec/h2645_sei.c
> +++ b/libavcodec/h2645_sei.c
> @@ -27,13 +27,13 @@
>  
>  #include "libavutil/ambient_viewing_environment.h"
>  #include "libavutil/display.h"
> +#include "libavutil/hdr_dynamic_metadata.h"
>  #include "libavutil/film_grain_params.h"
>  #include "libavutil/pixdesc.h"
>  #include "libavutil/stereo3d.h"
>  
>  #include "atsc_a53.h"
>  #include "avcodec.h"
> -#include "dynamic_hdr10_plus.h"
>  #include "dynamic_hdr_vivid.h"
>  #include "get_bits.h"
>  #include "golomb.h"
> @@ -52,8 +52,8 @@ static int decode_registered_user_data_dynamic_hdr_plus(HEVCSEIDynamicHDRPlus *s
>      if (!metadata)
>          return AVERROR(ENOMEM);
>  
> -    err = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(metadata, gb->buffer,
> -                                                   bytestream2_get_bytes_left(gb));
> +    err = av_dynamic_hdr_plus_from_t35(metadata, gb->buffer,
> +                                       bytestream2_get_bytes_left(gb));
>      if (err < 0) {
>          av_free(metadata);
>          return err;
> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> index eb1225ea1a..50c4ceee40 100644
> --- a/libavcodec/libdav1d.c
> +++ b/libavcodec/libdav1d.c
> @@ -24,6 +24,7 @@
>  #include "libavutil/avassert.h"
>  #include "libavutil/cpu.h"
>  #include "libavutil/film_grain_params.h"
> +#include "libavutil/hdr_dynamic_metadata.h"
>  #include "libavutil/mastering_display_metadata.h"
>  #include "libavutil/imgutils.h"
>  #include "libavutil/opt.h"
> @@ -33,7 +34,6 @@
>  #include "bytestream.h"
>  #include "codec_internal.h"
>  #include "decode.h"
> -#include "dynamic_hdr10_plus.h"
>  #include "internal.h"
>  
>  #define FF_DAV1D_VERSION_AT_LEAST(x,y) \
> @@ -555,8 +555,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
>                  goto fail;
>              }
>  
> -            res = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer,
> -                                                           bytestream2_get_bytes_left(&gb));
> +            res = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer,
> +                                               bytestream2_get_bytes_left(&gb));
>              if (res < 0)
>                  goto fail;
>              break;
> diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
> index 0fa1ee82de..5ed903f475 100644
> --- a/libavutil/hdr_dynamic_metadata.c
> +++ b/libavutil/hdr_dynamic_metadata.c
> @@ -20,6 +20,18 @@
>  
>  #include "hdr_dynamic_metadata.h"
>  #include "mem.h"
> +#include "libavcodec/get_bits.h"
> +#include "libavcodec/put_bits.h"
> +
> +#define T35_PAYLOAD_MAX_SIZE 907
> +
> +static const int64_t luminance_den = 1;
> +static const int32_t peak_luminance_den = 15;
> +static const int64_t rgb_den = 100000;
> +static const int32_t fraction_pixel_den = 1000;
> +static const int32_t knee_point_den = 4095;
> +static const int32_t bezier_anchor_den = 1023;
> +static const int32_t saturation_weight_den = 8;
>  
>  AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size)
>  {
> @@ -45,3 +57,176 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame)
>  
>      return (AVDynamicHDRPlus *)side_data->data;
>  }
> +
> +int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
> +                                 size_t size)
> +{
> +    uint8_t padded_buf[T35_PAYLOAD_MAX_SIZE + AV_INPUT_BUFFER_PADDING_SIZE];
> +    GetBitContext gbc, *gb = &gbc;
> +    int ret;
> +
> +    if (!s)
> +        return AVERROR(ENOMEM);
> +
> +    memcpy(padded_buf, data, size);

You are potentially copying more than T35_PAYLOAD_MAX_SIZE bytes.
Furthermore, you are not zeroing the padding; this might make Valgrind
complain.

> +
> +    ret = init_get_bits8(gb, padded_buf, size);
> +    if (ret < 0)
> +        return ret;
> +
> +    if (get_bits_left(gb) < 10)
> +        return AVERROR_INVALIDDATA;
> +
> +    s->application_version = get_bits(gb, 8);
> +    s->num_windows = get_bits(gb, 2);
> +
> +    if (s->num_windows < 1 || s->num_windows > 3) {
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    if (get_bits_left(gb) < ((19 * 8 + 1) * (s->num_windows - 1)))
> +        return AVERROR_INVALIDDATA;
> +
> +    for (int w = 1; w < s->num_windows; w++) {
> +        // The corners are set to absolute coordinates here. They should be
> +        // converted to the relative coordinates (in [0, 1]) in the decoder.
> +        AVHDRPlusColorTransformParams *params = &s->params[w];
> +        params->window_upper_left_corner_x =
> +            (AVRational){get_bits(gb, 16), 1};
> +        params->window_upper_left_corner_y =
> +            (AVRational){get_bits(gb, 16), 1};
> +        params->window_lower_right_corner_x =
> +            (AVRational){get_bits(gb, 16), 1};
> +        params->window_lower_right_corner_y =
> +            (AVRational){get_bits(gb, 16), 1};
> +
> +        params->center_of_ellipse_x = get_bits(gb, 16);
> +        params->center_of_ellipse_y = get_bits(gb, 16);
> +        params->rotation_angle = get_bits(gb, 8);
> +        params->semimajor_axis_internal_ellipse = get_bits(gb, 16);
> +        params->semimajor_axis_external_ellipse = get_bits(gb, 16);
> +        params->semiminor_axis_external_ellipse = get_bits(gb, 16);
> +        params->overlap_process_option = get_bits1(gb);
> +    }
> +
> +    if (get_bits_left(gb) < 28)
> +        return AVERROR_INVALIDDATA;
> +
> +    s->targeted_system_display_maximum_luminance =
> +        (AVRational){get_bits_long(gb, 27), luminance_den};
> +    s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
> +
> +    if (s->targeted_system_display_actual_peak_luminance_flag) {
> +        int rows, cols;
> +        if (get_bits_left(gb) < 10)
> +            return AVERROR_INVALIDDATA;
> +        rows = get_bits(gb, 5);
> +        cols = get_bits(gb, 5);
> +        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
> +            return AVERROR_INVALIDDATA;
> +        }
> +        s->num_rows_targeted_system_display_actual_peak_luminance = rows;
> +        s->num_cols_targeted_system_display_actual_peak_luminance = cols;
> +
> +        if (get_bits_left(gb) < (rows * cols * 4))
> +            return AVERROR_INVALIDDATA;
> +
> +        for (int i = 0; i < rows; i++) {
> +            for (int j = 0; j < cols; j++) {
> +                s->targeted_system_display_actual_peak_luminance[i][j] =
> +                    (AVRational){get_bits(gb, 4), peak_luminance_den};
> +            }
> +        }
> +    }
> +    for (int w = 0; w < s->num_windows; w++) {
> +        AVHDRPlusColorTransformParams *params = &s->params[w];
> +        if (get_bits_left(gb) < (3 * 17 + 17 + 4))
> +            return AVERROR_INVALIDDATA;
> +
> +        for (int i = 0; i < 3; i++) {
> +            params->maxscl[i] =
> +                (AVRational){get_bits(gb, 17), rgb_den};
> +        }
> +        params->average_maxrgb =
> +            (AVRational){get_bits(gb, 17), rgb_den};
> +        params->num_distribution_maxrgb_percentiles = get_bits(gb, 4);
> +
> +        if (get_bits_left(gb) <
> +            (params->num_distribution_maxrgb_percentiles * 24))
> +            return AVERROR_INVALIDDATA;
> +
> +        for (int i = 0; i < params->num_distribution_maxrgb_percentiles; i++) {
> +            params->distribution_maxrgb[i].percentage = get_bits(gb, 7);
> +            params->distribution_maxrgb[i].percentile =
> +                (AVRational){get_bits(gb, 17), rgb_den};
> +        }
> +
> +        if (get_bits_left(gb) < 10)
> +            return AVERROR_INVALIDDATA;
> +
> +        params->fraction_bright_pixels = (AVRational){get_bits(gb, 10), fraction_pixel_den};
> +    }
> +    if (get_bits_left(gb) < 1)
> +        return AVERROR_INVALIDDATA;
> +    s->mastering_display_actual_peak_luminance_flag = get_bits1(gb);
> +    if (s->mastering_display_actual_peak_luminance_flag) {
> +        int rows, cols;
> +        if (get_bits_left(gb) < 10)
> +            return AVERROR_INVALIDDATA;
> +        rows = get_bits(gb, 5);
> +        cols = get_bits(gb, 5);
> +        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
> +            return AVERROR_INVALIDDATA;
> +        }
> +        s->num_rows_mastering_display_actual_peak_luminance = rows;
> +        s->num_cols_mastering_display_actual_peak_luminance = cols;
> +
> +        if (get_bits_left(gb) < (rows * cols * 4))
> +            return AVERROR_INVALIDDATA;
> +
> +        for (int i = 0; i < rows; i++) {
> +            for (int j = 0; j < cols; j++) {
> +                s->mastering_display_actual_peak_luminance[i][j] =
> +                    (AVRational){get_bits(gb, 4), peak_luminance_den};
> +            }
> +        }
> +    }
> +
> +    for (int w = 0; w < s->num_windows; w++) {
> +        AVHDRPlusColorTransformParams *params = &s->params[w];
> +        if (get_bits_left(gb) < 1)
> +            return AVERROR_INVALIDDATA;
> +
> +        params->tone_mapping_flag = get_bits1(gb);
> +        if (params->tone_mapping_flag) {
> +            if (get_bits_left(gb) < 28)
> +                return AVERROR_INVALIDDATA;
> +
> +            params->knee_point_x =
> +                (AVRational){get_bits(gb, 12), knee_point_den};
> +            params->knee_point_y =
> +                (AVRational){get_bits(gb, 12), knee_point_den};
> +            params->num_bezier_curve_anchors = get_bits(gb, 4);
> +
> +            if (get_bits_left(gb) < (params->num_bezier_curve_anchors * 10))
> +                return AVERROR_INVALIDDATA;
> +
> +            for (int i = 0; i < params->num_bezier_curve_anchors; i++) {
> +                params->bezier_curve_anchors[i] =
> +                    (AVRational){get_bits(gb, 10), bezier_anchor_den};
> +            }
> +        }
> +
> +        if (get_bits_left(gb) < 1)
> +            return AVERROR_INVALIDDATA;
> +        params->color_saturation_mapping_flag = get_bits1(gb);
> +        if (params->color_saturation_mapping_flag) {
> +            if (get_bits_left(gb) < 6)
> +                return AVERROR_INVALIDDATA;
> +            params->color_saturation_weight =
> +                (AVRational){get_bits(gb, 6), saturation_weight_den};
> +        }
> +    }
> +
> +    return 0;
> +}
> diff --git a/libavutil/hdr_dynamic_metadata.h b/libavutil/hdr_dynamic_metadata.h
> index 2d72de56ae..faa4f46b0b 100644
> --- a/libavutil/hdr_dynamic_metadata.h
> +++ b/libavutil/hdr_dynamic_metadata.h
> @@ -340,4 +340,17 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size);
>   */
>  AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
>  
> +/**
> + * Parse the user data registered ITU-T T.35 to AVbuffer (AVDynamicHDRPlus).
> + * The T.35 buffer must begin with the application mode, skipping the
> + * country code, terminal provider codes, and application identifier.
> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
> + * @param data The byte array containing the raw ITU-T T.35 data.
> + * @param size Size of the data array in bytes.
> + *
> + * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
> + */
> +int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
> +                                 size_t size);
> +
>  #endif /* AVUTIL_HDR_DYNAMIC_METADATA_H */
Raphaël Zumer March 16, 2023, 8:37 p.m. UTC | #2
> On Mar 16, 2023, at 16:19, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
> 
> Raphaël Zumer:
>> Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
>> ---
>> libavcodec/Makefile              |   6 +-
>> libavcodec/av1dec.c              |   6 +-
>> libavcodec/dynamic_hdr10_plus.c  | 198 -------------------------------
>> libavcodec/dynamic_hdr10_plus.h  |  35 ------
>> libavcodec/h2645_sei.c           |   6 +-
>> libavcodec/libdav1d.c            |   6 +-
>> libavutil/hdr_dynamic_metadata.c | 185 +++++++++++++++++++++++++++++
>> libavutil/hdr_dynamic_metadata.h |  13 ++
>> 8 files changed, 210 insertions(+), 245 deletions(-)
>> delete mode 100644 libavcodec/dynamic_hdr10_plus.c
>> delete mode 100644 libavcodec/dynamic_hdr10_plus.h
>> 
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index abae4909d2..408ecd1e31 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -104,7 +104,7 @@ OBJS-$(CONFIG_H264_SEI)                += h264_sei.o h2645_sei.o
>> OBJS-$(CONFIG_HEVCPARSE)               += hevc_parse.o hevc_ps.o hevc_data.o \
>>                                           h2645data.o h2645_parse.o h2645_vui.o
>> OBJS-$(CONFIG_HEVC_SEI)                += hevc_sei.o h2645_sei.o \
>> -                                          dynamic_hdr10_plus.o dynamic_hdr_vivid.o
>> +                                          dynamic_hdr_vivid.o
>> OBJS-$(CONFIG_HPELDSP)                 += hpeldsp.o
>> OBJS-$(CONFIG_HUFFMAN)                 += huffman.o
>> OBJS-$(CONFIG_HUFFYUVDSP)              += huffyuvdsp.o
>> @@ -250,7 +250,7 @@ OBJS-$(CONFIG_ATRAC3PAL_DECODER)       += atrac3plusdec.o atrac3plus.o \
>> OBJS-$(CONFIG_ATRAC9_DECODER)          += atrac9dec.o
>> OBJS-$(CONFIG_AURA_DECODER)            += cyuv.o
>> OBJS-$(CONFIG_AURA2_DECODER)           += aura.o
>> -OBJS-$(CONFIG_AV1_DECODER)             += av1dec.o dynamic_hdr10_plus.o
>> +OBJS-$(CONFIG_AV1_DECODER)             += av1dec.o
>> OBJS-$(CONFIG_AV1_CUVID_DECODER)       += cuviddec.o
>> OBJS-$(CONFIG_AV1_MEDIACODEC_DECODER)  += mediacodecdec.o
>> OBJS-$(CONFIG_AV1_NVENC_ENCODER)       += nvenc_av1.o nvenc.o
>> @@ -1082,7 +1082,7 @@ OBJS-$(CONFIG_LIBARIBB24_DECODER)         += libaribb24.o ass.o
>> OBJS-$(CONFIG_LIBCELT_DECODER)            += libcelt_dec.o
>> OBJS-$(CONFIG_LIBCODEC2_DECODER)          += libcodec2.o
>> OBJS-$(CONFIG_LIBCODEC2_ENCODER)          += libcodec2.o
>> -OBJS-$(CONFIG_LIBDAV1D_DECODER)           += libdav1d.o dynamic_hdr10_plus.o
>> +OBJS-$(CONFIG_LIBDAV1D_DECODER)           += libdav1d.o
>> OBJS-$(CONFIG_LIBDAVS2_DECODER)           += libdavs2.o
>> OBJS-$(CONFIG_LIBFDK_AAC_DECODER)         += libfdk-aacdec.o
>> OBJS-$(CONFIG_LIBFDK_AAC_ENCODER)         += libfdk-aacenc.o
>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>> index a80e37e33f..df393fe3d0 100644
>> --- a/libavcodec/av1dec.c
>> +++ b/libavcodec/av1dec.c
>> @@ -20,6 +20,7 @@
>> 
>> #include "config_components.h"
>> 
>> +#include "libavutil/hdr_dynamic_metadata.h"
>> #include "libavutil/film_grain_params.h"
>> #include "libavutil/mastering_display_metadata.h"
>> #include "libavutil/pixdesc.h"
>> @@ -30,7 +31,6 @@
>> #include "bytestream.h"
>> #include "codec_internal.h"
>> #include "decode.h"
>> -#include "dynamic_hdr10_plus.h"
>> #include "hwconfig.h"
>> #include "profiles.h"
>> #include "thread.h"
>> @@ -925,8 +925,8 @@ static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame,
>>         if (!hdrplus)
>>             return AVERROR(ENOMEM);
>> 
>> -        ret = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer,
>> -                                                       bytestream2_get_bytes_left(&gb));
>> +        ret = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer,
>> +                                           bytestream2_get_bytes_left(&gb));
>>         if (ret < 0)
>>             return ret;
>>         break;
>> diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c
>> deleted file mode 100644
>> index 34a44aac65..0000000000
>> --- a/libavcodec/dynamic_hdr10_plus.c
>> +++ /dev/null
>> @@ -1,198 +0,0 @@
>> -/*
>> - * This file is part of FFmpeg.
>> - *
>> - * FFmpeg is free software; you can redistribute it and/or
>> - * modify it under the terms of the GNU Lesser General Public
>> - * License as published by the Free Software Foundation; either
>> - * version 2.1 of the License, or (at your option) any later version.
>> - *
>> - * FFmpeg is distributed in the hope that it will be useful,
>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> - * Lesser General Public License for more details.
>> - *
>> - * You should have received a copy of the GNU Lesser General Public
>> - * License along with FFmpeg; if not, write to the Free Software
>> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> - */
>> -
>> -#include "dynamic_hdr10_plus.h"
>> -#include "get_bits.h"
>> -
>> -static const int64_t luminance_den = 1;
>> -static const int32_t peak_luminance_den = 15;
>> -static const int64_t rgb_den = 100000;
>> -static const int32_t fraction_pixel_den = 1000;
>> -static const int32_t knee_point_den = 4095;
>> -static const int32_t bezier_anchor_den = 1023;
>> -static const int32_t saturation_weight_den = 8;
>> -
>> -int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
>> -                                             int size)
>> -{
>> -    GetBitContext gbc, *gb = &gbc;
>> -    int ret;
>> -
>> -    if (!s)
>> -        return AVERROR(ENOMEM);
>> -
>> -    ret = init_get_bits8(gb, data, size);
>> -    if (ret < 0)
>> -        return ret;
>> -
>> -    if (get_bits_left(gb) < 10)
>> -        return AVERROR_INVALIDDATA;
>> -
>> -    s->application_version = get_bits(gb, 8);
>> -    s->num_windows = get_bits(gb, 2);
>> -
>> -    if (s->num_windows < 1 || s->num_windows > 3) {
>> -        return AVERROR_INVALIDDATA;
>> -    }
>> -
>> -    if (get_bits_left(gb) < ((19 * 8 + 1) * (s->num_windows - 1)))
>> -        return AVERROR_INVALIDDATA;
>> -
>> -    for (int w = 1; w < s->num_windows; w++) {
>> -        // The corners are set to absolute coordinates here. They should be
>> -        // converted to the relative coordinates (in [0, 1]) in the decoder.
>> -        AVHDRPlusColorTransformParams *params = &s->params[w];
>> -        params->window_upper_left_corner_x =
>> -            (AVRational){get_bits(gb, 16), 1};
>> -        params->window_upper_left_corner_y =
>> -            (AVRational){get_bits(gb, 16), 1};
>> -        params->window_lower_right_corner_x =
>> -            (AVRational){get_bits(gb, 16), 1};
>> -        params->window_lower_right_corner_y =
>> -            (AVRational){get_bits(gb, 16), 1};
>> -
>> -        params->center_of_ellipse_x = get_bits(gb, 16);
>> -        params->center_of_ellipse_y = get_bits(gb, 16);
>> -        params->rotation_angle = get_bits(gb, 8);
>> -        params->semimajor_axis_internal_ellipse = get_bits(gb, 16);
>> -        params->semimajor_axis_external_ellipse = get_bits(gb, 16);
>> -        params->semiminor_axis_external_ellipse = get_bits(gb, 16);
>> -        params->overlap_process_option = get_bits1(gb);
>> -    }
>> -
>> -    if (get_bits_left(gb) < 28)
>> -        return AVERROR_INVALIDDATA;
>> -
>> -    s->targeted_system_display_maximum_luminance =
>> -        (AVRational){get_bits_long(gb, 27), luminance_den};
>> -    s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
>> -
>> -    if (s->targeted_system_display_actual_peak_luminance_flag) {
>> -        int rows, cols;
>> -        if (get_bits_left(gb) < 10)
>> -            return AVERROR_INVALIDDATA;
>> -        rows = get_bits(gb, 5);
>> -        cols = get_bits(gb, 5);
>> -        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
>> -            return AVERROR_INVALIDDATA;
>> -        }
>> -        s->num_rows_targeted_system_display_actual_peak_luminance = rows;
>> -        s->num_cols_targeted_system_display_actual_peak_luminance = cols;
>> -
>> -        if (get_bits_left(gb) < (rows * cols * 4))
>> -            return AVERROR_INVALIDDATA;
>> -
>> -        for (int i = 0; i < rows; i++) {
>> -            for (int j = 0; j < cols; j++) {
>> -                s->targeted_system_display_actual_peak_luminance[i][j] =
>> -                    (AVRational){get_bits(gb, 4), peak_luminance_den};
>> -            }
>> -        }
>> -    }
>> -    for (int w = 0; w < s->num_windows; w++) {
>> -        AVHDRPlusColorTransformParams *params = &s->params[w];
>> -        if (get_bits_left(gb) < (3 * 17 + 17 + 4))
>> -            return AVERROR_INVALIDDATA;
>> -
>> -        for (int i = 0; i < 3; i++) {
>> -            params->maxscl[i] =
>> -                (AVRational){get_bits(gb, 17), rgb_den};
>> -        }
>> -        params->average_maxrgb =
>> -            (AVRational){get_bits(gb, 17), rgb_den};
>> -        params->num_distribution_maxrgb_percentiles = get_bits(gb, 4);
>> -
>> -        if (get_bits_left(gb) <
>> -            (params->num_distribution_maxrgb_percentiles * 24))
>> -            return AVERROR_INVALIDDATA;
>> -
>> -        for (int i = 0; i < params->num_distribution_maxrgb_percentiles; i++) {
>> -            params->distribution_maxrgb[i].percentage = get_bits(gb, 7);
>> -            params->distribution_maxrgb[i].percentile =
>> -                (AVRational){get_bits(gb, 17), rgb_den};
>> -        }
>> -
>> -        if (get_bits_left(gb) < 10)
>> -            return AVERROR_INVALIDDATA;
>> -
>> -        params->fraction_bright_pixels = (AVRational){get_bits(gb, 10), fraction_pixel_den};
>> -    }
>> -    if (get_bits_left(gb) < 1)
>> -        return AVERROR_INVALIDDATA;
>> -    s->mastering_display_actual_peak_luminance_flag = get_bits1(gb);
>> -    if (s->mastering_display_actual_peak_luminance_flag) {
>> -        int rows, cols;
>> -        if (get_bits_left(gb) < 10)
>> -            return AVERROR_INVALIDDATA;
>> -        rows = get_bits(gb, 5);
>> -        cols = get_bits(gb, 5);
>> -        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
>> -            return AVERROR_INVALIDDATA;
>> -        }
>> -        s->num_rows_mastering_display_actual_peak_luminance = rows;
>> -        s->num_cols_mastering_display_actual_peak_luminance = cols;
>> -
>> -        if (get_bits_left(gb) < (rows * cols * 4))
>> -            return AVERROR_INVALIDDATA;
>> -
>> -        for (int i = 0; i < rows; i++) {
>> -            for (int j = 0; j < cols; j++) {
>> -                s->mastering_display_actual_peak_luminance[i][j] =
>> -                    (AVRational){get_bits(gb, 4), peak_luminance_den};
>> -            }
>> -        }
>> -    }
>> -
>> -    for (int w = 0; w < s->num_windows; w++) {
>> -        AVHDRPlusColorTransformParams *params = &s->params[w];
>> -        if (get_bits_left(gb) < 1)
>> -            return AVERROR_INVALIDDATA;
>> -
>> -        params->tone_mapping_flag = get_bits1(gb);
>> -        if (params->tone_mapping_flag) {
>> -            if (get_bits_left(gb) < 28)
>> -                return AVERROR_INVALIDDATA;
>> -
>> -            params->knee_point_x =
>> -                (AVRational){get_bits(gb, 12), knee_point_den};
>> -            params->knee_point_y =
>> -                (AVRational){get_bits(gb, 12), knee_point_den};
>> -            params->num_bezier_curve_anchors = get_bits(gb, 4);
>> -
>> -            if (get_bits_left(gb) < (params->num_bezier_curve_anchors * 10))
>> -                return AVERROR_INVALIDDATA;
>> -
>> -            for (int i = 0; i < params->num_bezier_curve_anchors; i++) {
>> -                params->bezier_curve_anchors[i] =
>> -                    (AVRational){get_bits(gb, 10), bezier_anchor_den};
>> -            }
>> -        }
>> -
>> -        if (get_bits_left(gb) < 1)
>> -            return AVERROR_INVALIDDATA;
>> -        params->color_saturation_mapping_flag = get_bits1(gb);
>> -        if (params->color_saturation_mapping_flag) {
>> -            if (get_bits_left(gb) < 6)
>> -                return AVERROR_INVALIDDATA;
>> -            params->color_saturation_weight =
>> -                (AVRational){get_bits(gb, 6), saturation_weight_den};
>> -        }
>> -    }
>> -
>> -    return 0;
>> -}
>> diff --git a/libavcodec/dynamic_hdr10_plus.h b/libavcodec/dynamic_hdr10_plus.h
>> deleted file mode 100644
>> index cd7acf0432..0000000000
>> --- a/libavcodec/dynamic_hdr10_plus.h
>> +++ /dev/null
>> @@ -1,35 +0,0 @@
>> -/*
>> - * This file is part of FFmpeg.
>> - *
>> - * FFmpeg is free software; you can redistribute it and/or
>> - * modify it under the terms of the GNU Lesser General Public
>> - * License as published by the Free Software Foundation; either
>> - * version 2.1 of the License, or (at your option) any later version.
>> - *
>> - * FFmpeg is distributed in the hope that it will be useful,
>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> - * Lesser General Public License for more details.
>> - *
>> - * You should have received a copy of the GNU Lesser General Public
>> - * License along with FFmpeg; if not, write to the Free Software
>> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> - */
>> -
>> -#ifndef AVCODEC_DYNAMIC_HDR10_PLUS_H
>> -#define AVCODEC_DYNAMIC_HDR10_PLUS_H
>> -
>> -#include "libavutil/hdr_dynamic_metadata.h"
>> -
>> -/**
>> - * Parse the user data registered ITU-T T.35 to AVbuffer (AVDynamicHDRPlus).
>> - * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
>> - * @param data The byte array containing the raw ITU-T T.35 data.
>> - * @param size Size of the data array in bytes.
>> - *
>> - * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
>> - */
>> -int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
>> -                                             int size);
>> -
>> -#endif /* AVCODEC_DYNAMIC_HDR10_PLUS_H */
>> diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
>> index 6e4a9a1af2..63ab711bc9 100644
>> --- a/libavcodec/h2645_sei.c
>> +++ b/libavcodec/h2645_sei.c
>> @@ -27,13 +27,13 @@
>> 
>> #include "libavutil/ambient_viewing_environment.h"
>> #include "libavutil/display.h"
>> +#include "libavutil/hdr_dynamic_metadata.h"
>> #include "libavutil/film_grain_params.h"
>> #include "libavutil/pixdesc.h"
>> #include "libavutil/stereo3d.h"
>> 
>> #include "atsc_a53.h"
>> #include "avcodec.h"
>> -#include "dynamic_hdr10_plus.h"
>> #include "dynamic_hdr_vivid.h"
>> #include "get_bits.h"
>> #include "golomb.h"
>> @@ -52,8 +52,8 @@ static int decode_registered_user_data_dynamic_hdr_plus(HEVCSEIDynamicHDRPlus *s
>>     if (!metadata)
>>         return AVERROR(ENOMEM);
>> 
>> -    err = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(metadata, gb->buffer,
>> -                                                   bytestream2_get_bytes_left(gb));
>> +    err = av_dynamic_hdr_plus_from_t35(metadata, gb->buffer,
>> +                                       bytestream2_get_bytes_left(gb));
>>     if (err < 0) {
>>         av_free(metadata);
>>         return err;
>> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
>> index eb1225ea1a..50c4ceee40 100644
>> --- a/libavcodec/libdav1d.c
>> +++ b/libavcodec/libdav1d.c
>> @@ -24,6 +24,7 @@
>> #include "libavutil/avassert.h"
>> #include "libavutil/cpu.h"
>> #include "libavutil/film_grain_params.h"
>> +#include "libavutil/hdr_dynamic_metadata.h"
>> #include "libavutil/mastering_display_metadata.h"
>> #include "libavutil/imgutils.h"
>> #include "libavutil/opt.h"
>> @@ -33,7 +34,6 @@
>> #include "bytestream.h"
>> #include "codec_internal.h"
>> #include "decode.h"
>> -#include "dynamic_hdr10_plus.h"
>> #include "internal.h"
>> 
>> #define FF_DAV1D_VERSION_AT_LEAST(x,y) \
>> @@ -555,8 +555,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>                 goto fail;
>>             }
>> 
>> -            res = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer,
>> -                                                           bytestream2_get_bytes_left(&gb));
>> +            res = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer,
>> +                                               bytestream2_get_bytes_left(&gb));
>>             if (res < 0)
>>                 goto fail;
>>             break;
>> diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
>> index 0fa1ee82de..5ed903f475 100644
>> --- a/libavutil/hdr_dynamic_metadata.c
>> +++ b/libavutil/hdr_dynamic_metadata.c
>> @@ -20,6 +20,18 @@
>> 
>> #include "hdr_dynamic_metadata.h"
>> #include "mem.h"
>> +#include "libavcodec/get_bits.h"
>> +#include "libavcodec/put_bits.h"
>> +
>> +#define T35_PAYLOAD_MAX_SIZE 907
>> +
>> +static const int64_t luminance_den = 1;
>> +static const int32_t peak_luminance_den = 15;
>> +static const int64_t rgb_den = 100000;
>> +static const int32_t fraction_pixel_den = 1000;
>> +static const int32_t knee_point_den = 4095;
>> +static const int32_t bezier_anchor_den = 1023;
>> +static const int32_t saturation_weight_den = 8;
>> 
>> AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size)
>> {
>> @@ -45,3 +57,176 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame)
>> 
>>     return (AVDynamicHDRPlus *)side_data->data;
>> }
>> +
>> +int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>> +                                 size_t size)
>> +{
>> +    uint8_t padded_buf[T35_PAYLOAD_MAX_SIZE + AV_INPUT_BUFFER_PADDING_SIZE];
>> +    GetBitContext gbc, *gb = &gbc;
>> +    int ret;
>> +
>> +    if (!s)
>> +        return AVERROR(ENOMEM);
>> +
>> +    memcpy(padded_buf, data, size);
> 
> You are potentially copying more than T35_PAYLOAD_MAX_SIZE bytes.
> Furthermore, you are not zeroing the padding; this might make Valgrind
> complain.

Good point on the size if the user sends an incorrect value, we can return AVERROR(EINVAL) in that case. I did run it through Valgrind and did not get an error due to not zeroing the padded buffer. I don’t think that should be necessary since it’s contained within that function and uninitialized bits should never be accessed, is that incorrect?

RZ

> 
>> +
>> +    ret = init_get_bits8(gb, padded_buf, size);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    if (get_bits_left(gb) < 10)
>> +        return AVERROR_INVALIDDATA;
>> +
>> +    s->application_version = get_bits(gb, 8);
>> +    s->num_windows = get_bits(gb, 2);
>> +
>> +    if (s->num_windows < 1 || s->num_windows > 3) {
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +
>> +    if (get_bits_left(gb) < ((19 * 8 + 1) * (s->num_windows - 1)))
>> +        return AVERROR_INVALIDDATA;
>> +
>> +    for (int w = 1; w < s->num_windows; w++) {
>> +        // The corners are set to absolute coordinates here. They should be
>> +        // converted to the relative coordinates (in [0, 1]) in the decoder.
>> +        AVHDRPlusColorTransformParams *params = &s->params[w];
>> +        params->window_upper_left_corner_x =
>> +            (AVRational){get_bits(gb, 16), 1};
>> +        params->window_upper_left_corner_y =
>> +            (AVRational){get_bits(gb, 16), 1};
>> +        params->window_lower_right_corner_x =
>> +            (AVRational){get_bits(gb, 16), 1};
>> +        params->window_lower_right_corner_y =
>> +            (AVRational){get_bits(gb, 16), 1};
>> +
>> +        params->center_of_ellipse_x = get_bits(gb, 16);
>> +        params->center_of_ellipse_y = get_bits(gb, 16);
>> +        params->rotation_angle = get_bits(gb, 8);
>> +        params->semimajor_axis_internal_ellipse = get_bits(gb, 16);
>> +        params->semimajor_axis_external_ellipse = get_bits(gb, 16);
>> +        params->semiminor_axis_external_ellipse = get_bits(gb, 16);
>> +        params->overlap_process_option = get_bits1(gb);
>> +    }
>> +
>> +    if (get_bits_left(gb) < 28)
>> +        return AVERROR_INVALIDDATA;
>> +
>> +    s->targeted_system_display_maximum_luminance =
>> +        (AVRational){get_bits_long(gb, 27), luminance_den};
>> +    s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
>> +
>> +    if (s->targeted_system_display_actual_peak_luminance_flag) {
>> +        int rows, cols;
>> +        if (get_bits_left(gb) < 10)
>> +            return AVERROR_INVALIDDATA;
>> +        rows = get_bits(gb, 5);
>> +        cols = get_bits(gb, 5);
>> +        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
>> +            return AVERROR_INVALIDDATA;
>> +        }
>> +        s->num_rows_targeted_system_display_actual_peak_luminance = rows;
>> +        s->num_cols_targeted_system_display_actual_peak_luminance = cols;
>> +
>> +        if (get_bits_left(gb) < (rows * cols * 4))
>> +            return AVERROR_INVALIDDATA;
>> +
>> +        for (int i = 0; i < rows; i++) {
>> +            for (int j = 0; j < cols; j++) {
>> +                s->targeted_system_display_actual_peak_luminance[i][j] =
>> +                    (AVRational){get_bits(gb, 4), peak_luminance_den};
>> +            }
>> +        }
>> +    }
>> +    for (int w = 0; w < s->num_windows; w++) {
>> +        AVHDRPlusColorTransformParams *params = &s->params[w];
>> +        if (get_bits_left(gb) < (3 * 17 + 17 + 4))
>> +            return AVERROR_INVALIDDATA;
>> +
>> +        for (int i = 0; i < 3; i++) {
>> +            params->maxscl[i] =
>> +                (AVRational){get_bits(gb, 17), rgb_den};
>> +        }
>> +        params->average_maxrgb =
>> +            (AVRational){get_bits(gb, 17), rgb_den};
>> +        params->num_distribution_maxrgb_percentiles = get_bits(gb, 4);
>> +
>> +        if (get_bits_left(gb) <
>> +            (params->num_distribution_maxrgb_percentiles * 24))
>> +            return AVERROR_INVALIDDATA;
>> +
>> +        for (int i = 0; i < params->num_distribution_maxrgb_percentiles; i++) {
>> +            params->distribution_maxrgb[i].percentage = get_bits(gb, 7);
>> +            params->distribution_maxrgb[i].percentile =
>> +                (AVRational){get_bits(gb, 17), rgb_den};
>> +        }
>> +
>> +        if (get_bits_left(gb) < 10)
>> +            return AVERROR_INVALIDDATA;
>> +
>> +        params->fraction_bright_pixels = (AVRational){get_bits(gb, 10), fraction_pixel_den};
>> +    }
>> +    if (get_bits_left(gb) < 1)
>> +        return AVERROR_INVALIDDATA;
>> +    s->mastering_display_actual_peak_luminance_flag = get_bits1(gb);
>> +    if (s->mastering_display_actual_peak_luminance_flag) {
>> +        int rows, cols;
>> +        if (get_bits_left(gb) < 10)
>> +            return AVERROR_INVALIDDATA;
>> +        rows = get_bits(gb, 5);
>> +        cols = get_bits(gb, 5);
>> +        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
>> +            return AVERROR_INVALIDDATA;
>> +        }
>> +        s->num_rows_mastering_display_actual_peak_luminance = rows;
>> +        s->num_cols_mastering_display_actual_peak_luminance = cols;
>> +
>> +        if (get_bits_left(gb) < (rows * cols * 4))
>> +            return AVERROR_INVALIDDATA;
>> +
>> +        for (int i = 0; i < rows; i++) {
>> +            for (int j = 0; j < cols; j++) {
>> +                s->mastering_display_actual_peak_luminance[i][j] =
>> +                    (AVRational){get_bits(gb, 4), peak_luminance_den};
>> +            }
>> +        }
>> +    }
>> +
>> +    for (int w = 0; w < s->num_windows; w++) {
>> +        AVHDRPlusColorTransformParams *params = &s->params[w];
>> +        if (get_bits_left(gb) < 1)
>> +            return AVERROR_INVALIDDATA;
>> +
>> +        params->tone_mapping_flag = get_bits1(gb);
>> +        if (params->tone_mapping_flag) {
>> +            if (get_bits_left(gb) < 28)
>> +                return AVERROR_INVALIDDATA;
>> +
>> +            params->knee_point_x =
>> +                (AVRational){get_bits(gb, 12), knee_point_den};
>> +            params->knee_point_y =
>> +                (AVRational){get_bits(gb, 12), knee_point_den};
>> +            params->num_bezier_curve_anchors = get_bits(gb, 4);
>> +
>> +            if (get_bits_left(gb) < (params->num_bezier_curve_anchors * 10))
>> +                return AVERROR_INVALIDDATA;
>> +
>> +            for (int i = 0; i < params->num_bezier_curve_anchors; i++) {
>> +                params->bezier_curve_anchors[i] =
>> +                    (AVRational){get_bits(gb, 10), bezier_anchor_den};
>> +            }
>> +        }
>> +
>> +        if (get_bits_left(gb) < 1)
>> +            return AVERROR_INVALIDDATA;
>> +        params->color_saturation_mapping_flag = get_bits1(gb);
>> +        if (params->color_saturation_mapping_flag) {
>> +            if (get_bits_left(gb) < 6)
>> +                return AVERROR_INVALIDDATA;
>> +            params->color_saturation_weight =
>> +                (AVRational){get_bits(gb, 6), saturation_weight_den};
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> diff --git a/libavutil/hdr_dynamic_metadata.h b/libavutil/hdr_dynamic_metadata.h
>> index 2d72de56ae..faa4f46b0b 100644
>> --- a/libavutil/hdr_dynamic_metadata.h
>> +++ b/libavutil/hdr_dynamic_metadata.h
>> @@ -340,4 +340,17 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size);
>>  */
>> AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
>> 
>> +/**
>> + * Parse the user data registered ITU-T T.35 to AVbuffer (AVDynamicHDRPlus).
>> + * The T.35 buffer must begin with the application mode, skipping the
>> + * country code, terminal provider codes, and application identifier.
>> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
>> + * @param data The byte array containing the raw ITU-T T.35 data.
>> + * @param size Size of the data array in bytes.
>> + *
>> + * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
>> + */
>> +int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>> +                                 size_t size);
>> +
>> #endif /* AVUTIL_HDR_DYNAMIC_METADATA_H */
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Andreas Rheinhardt March 16, 2023, 8:50 p.m. UTC | #3
Raphaël Zumer:
> 
> 
>> On Mar 16, 2023, at 16:19, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
>>
>> Raphaël Zumer:
>>> Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
>>> ---
>>> libavcodec/Makefile              |   6 +-
>>> libavcodec/av1dec.c              |   6 +-
>>> libavcodec/dynamic_hdr10_plus.c  | 198 -------------------------------
>>> libavcodec/dynamic_hdr10_plus.h  |  35 ------
>>> libavcodec/h2645_sei.c           |   6 +-
>>> libavcodec/libdav1d.c            |   6 +-
>>> libavutil/hdr_dynamic_metadata.c | 185 +++++++++++++++++++++++++++++
>>> libavutil/hdr_dynamic_metadata.h |  13 ++
>>> 8 files changed, 210 insertions(+), 245 deletions(-)
>>> delete mode 100644 libavcodec/dynamic_hdr10_plus.c
>>> delete mode 100644 libavcodec/dynamic_hdr10_plus.h
>>>
>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>>> index abae4909d2..408ecd1e31 100644
>>> --- a/libavcodec/Makefile
>>> +++ b/libavcodec/Makefile
>>> @@ -104,7 +104,7 @@ OBJS-$(CONFIG_H264_SEI)                += h264_sei.o h2645_sei.o
>>> OBJS-$(CONFIG_HEVCPARSE)               += hevc_parse.o hevc_ps.o hevc_data.o \
>>>                                           h2645data.o h2645_parse.o h2645_vui.o
>>> OBJS-$(CONFIG_HEVC_SEI)                += hevc_sei.o h2645_sei.o \
>>> -                                          dynamic_hdr10_plus.o dynamic_hdr_vivid.o
>>> +                                          dynamic_hdr_vivid.o
>>> OBJS-$(CONFIG_HPELDSP)                 += hpeldsp.o
>>> OBJS-$(CONFIG_HUFFMAN)                 += huffman.o
>>> OBJS-$(CONFIG_HUFFYUVDSP)              += huffyuvdsp.o
>>> @@ -250,7 +250,7 @@ OBJS-$(CONFIG_ATRAC3PAL_DECODER)       += atrac3plusdec.o atrac3plus.o \
>>> OBJS-$(CONFIG_ATRAC9_DECODER)          += atrac9dec.o
>>> OBJS-$(CONFIG_AURA_DECODER)            += cyuv.o
>>> OBJS-$(CONFIG_AURA2_DECODER)           += aura.o
>>> -OBJS-$(CONFIG_AV1_DECODER)             += av1dec.o dynamic_hdr10_plus.o
>>> +OBJS-$(CONFIG_AV1_DECODER)             += av1dec.o
>>> OBJS-$(CONFIG_AV1_CUVID_DECODER)       += cuviddec.o
>>> OBJS-$(CONFIG_AV1_MEDIACODEC_DECODER)  += mediacodecdec.o
>>> OBJS-$(CONFIG_AV1_NVENC_ENCODER)       += nvenc_av1.o nvenc.o
>>> @@ -1082,7 +1082,7 @@ OBJS-$(CONFIG_LIBARIBB24_DECODER)         += libaribb24.o ass.o
>>> OBJS-$(CONFIG_LIBCELT_DECODER)            += libcelt_dec.o
>>> OBJS-$(CONFIG_LIBCODEC2_DECODER)          += libcodec2.o
>>> OBJS-$(CONFIG_LIBCODEC2_ENCODER)          += libcodec2.o
>>> -OBJS-$(CONFIG_LIBDAV1D_DECODER)           += libdav1d.o dynamic_hdr10_plus.o
>>> +OBJS-$(CONFIG_LIBDAV1D_DECODER)           += libdav1d.o
>>> OBJS-$(CONFIG_LIBDAVS2_DECODER)           += libdavs2.o
>>> OBJS-$(CONFIG_LIBFDK_AAC_DECODER)         += libfdk-aacdec.o
>>> OBJS-$(CONFIG_LIBFDK_AAC_ENCODER)         += libfdk-aacenc.o
>>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>>> index a80e37e33f..df393fe3d0 100644
>>> --- a/libavcodec/av1dec.c
>>> +++ b/libavcodec/av1dec.c
>>> @@ -20,6 +20,7 @@
>>>
>>> #include "config_components.h"
>>>
>>> +#include "libavutil/hdr_dynamic_metadata.h"
>>> #include "libavutil/film_grain_params.h"
>>> #include "libavutil/mastering_display_metadata.h"
>>> #include "libavutil/pixdesc.h"
>>> @@ -30,7 +31,6 @@
>>> #include "bytestream.h"
>>> #include "codec_internal.h"
>>> #include "decode.h"
>>> -#include "dynamic_hdr10_plus.h"
>>> #include "hwconfig.h"
>>> #include "profiles.h"
>>> #include "thread.h"
>>> @@ -925,8 +925,8 @@ static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame,
>>>         if (!hdrplus)
>>>             return AVERROR(ENOMEM);
>>>
>>> -        ret = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer,
>>> -                                                       bytestream2_get_bytes_left(&gb));
>>> +        ret = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer,
>>> +                                           bytestream2_get_bytes_left(&gb));
>>>         if (ret < 0)
>>>             return ret;
>>>         break;
>>> diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c
>>> deleted file mode 100644
>>> index 34a44aac65..0000000000
>>> --- a/libavcodec/dynamic_hdr10_plus.c
>>> +++ /dev/null
>>> @@ -1,198 +0,0 @@
>>> -/*
>>> - * This file is part of FFmpeg.
>>> - *
>>> - * FFmpeg is free software; you can redistribute it and/or
>>> - * modify it under the terms of the GNU Lesser General Public
>>> - * License as published by the Free Software Foundation; either
>>> - * version 2.1 of the License, or (at your option) any later version.
>>> - *
>>> - * FFmpeg is distributed in the hope that it will be useful,
>>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> - * Lesser General Public License for more details.
>>> - *
>>> - * You should have received a copy of the GNU Lesser General Public
>>> - * License along with FFmpeg; if not, write to the Free Software
>>> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>>> - */
>>> -
>>> -#include "dynamic_hdr10_plus.h"
>>> -#include "get_bits.h"
>>> -
>>> -static const int64_t luminance_den = 1;
>>> -static const int32_t peak_luminance_den = 15;
>>> -static const int64_t rgb_den = 100000;
>>> -static const int32_t fraction_pixel_den = 1000;
>>> -static const int32_t knee_point_den = 4095;
>>> -static const int32_t bezier_anchor_den = 1023;
>>> -static const int32_t saturation_weight_den = 8;
>>> -
>>> -int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
>>> -                                             int size)
>>> -{
>>> -    GetBitContext gbc, *gb = &gbc;
>>> -    int ret;
>>> -
>>> -    if (!s)
>>> -        return AVERROR(ENOMEM);
>>> -
>>> -    ret = init_get_bits8(gb, data, size);
>>> -    if (ret < 0)
>>> -        return ret;
>>> -
>>> -    if (get_bits_left(gb) < 10)
>>> -        return AVERROR_INVALIDDATA;
>>> -
>>> -    s->application_version = get_bits(gb, 8);
>>> -    s->num_windows = get_bits(gb, 2);
>>> -
>>> -    if (s->num_windows < 1 || s->num_windows > 3) {
>>> -        return AVERROR_INVALIDDATA;
>>> -    }
>>> -
>>> -    if (get_bits_left(gb) < ((19 * 8 + 1) * (s->num_windows - 1)))
>>> -        return AVERROR_INVALIDDATA;
>>> -
>>> -    for (int w = 1; w < s->num_windows; w++) {
>>> -        // The corners are set to absolute coordinates here. They should be
>>> -        // converted to the relative coordinates (in [0, 1]) in the decoder.
>>> -        AVHDRPlusColorTransformParams *params = &s->params[w];
>>> -        params->window_upper_left_corner_x =
>>> -            (AVRational){get_bits(gb, 16), 1};
>>> -        params->window_upper_left_corner_y =
>>> -            (AVRational){get_bits(gb, 16), 1};
>>> -        params->window_lower_right_corner_x =
>>> -            (AVRational){get_bits(gb, 16), 1};
>>> -        params->window_lower_right_corner_y =
>>> -            (AVRational){get_bits(gb, 16), 1};
>>> -
>>> -        params->center_of_ellipse_x = get_bits(gb, 16);
>>> -        params->center_of_ellipse_y = get_bits(gb, 16);
>>> -        params->rotation_angle = get_bits(gb, 8);
>>> -        params->semimajor_axis_internal_ellipse = get_bits(gb, 16);
>>> -        params->semimajor_axis_external_ellipse = get_bits(gb, 16);
>>> -        params->semiminor_axis_external_ellipse = get_bits(gb, 16);
>>> -        params->overlap_process_option = get_bits1(gb);
>>> -    }
>>> -
>>> -    if (get_bits_left(gb) < 28)
>>> -        return AVERROR_INVALIDDATA;
>>> -
>>> -    s->targeted_system_display_maximum_luminance =
>>> -        (AVRational){get_bits_long(gb, 27), luminance_den};
>>> -    s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
>>> -
>>> -    if (s->targeted_system_display_actual_peak_luminance_flag) {
>>> -        int rows, cols;
>>> -        if (get_bits_left(gb) < 10)
>>> -            return AVERROR_INVALIDDATA;
>>> -        rows = get_bits(gb, 5);
>>> -        cols = get_bits(gb, 5);
>>> -        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
>>> -            return AVERROR_INVALIDDATA;
>>> -        }
>>> -        s->num_rows_targeted_system_display_actual_peak_luminance = rows;
>>> -        s->num_cols_targeted_system_display_actual_peak_luminance = cols;
>>> -
>>> -        if (get_bits_left(gb) < (rows * cols * 4))
>>> -            return AVERROR_INVALIDDATA;
>>> -
>>> -        for (int i = 0; i < rows; i++) {
>>> -            for (int j = 0; j < cols; j++) {
>>> -                s->targeted_system_display_actual_peak_luminance[i][j] =
>>> -                    (AVRational){get_bits(gb, 4), peak_luminance_den};
>>> -            }
>>> -        }
>>> -    }
>>> -    for (int w = 0; w < s->num_windows; w++) {
>>> -        AVHDRPlusColorTransformParams *params = &s->params[w];
>>> -        if (get_bits_left(gb) < (3 * 17 + 17 + 4))
>>> -            return AVERROR_INVALIDDATA;
>>> -
>>> -        for (int i = 0; i < 3; i++) {
>>> -            params->maxscl[i] =
>>> -                (AVRational){get_bits(gb, 17), rgb_den};
>>> -        }
>>> -        params->average_maxrgb =
>>> -            (AVRational){get_bits(gb, 17), rgb_den};
>>> -        params->num_distribution_maxrgb_percentiles = get_bits(gb, 4);
>>> -
>>> -        if (get_bits_left(gb) <
>>> -            (params->num_distribution_maxrgb_percentiles * 24))
>>> -            return AVERROR_INVALIDDATA;
>>> -
>>> -        for (int i = 0; i < params->num_distribution_maxrgb_percentiles; i++) {
>>> -            params->distribution_maxrgb[i].percentage = get_bits(gb, 7);
>>> -            params->distribution_maxrgb[i].percentile =
>>> -                (AVRational){get_bits(gb, 17), rgb_den};
>>> -        }
>>> -
>>> -        if (get_bits_left(gb) < 10)
>>> -            return AVERROR_INVALIDDATA;
>>> -
>>> -        params->fraction_bright_pixels = (AVRational){get_bits(gb, 10), fraction_pixel_den};
>>> -    }
>>> -    if (get_bits_left(gb) < 1)
>>> -        return AVERROR_INVALIDDATA;
>>> -    s->mastering_display_actual_peak_luminance_flag = get_bits1(gb);
>>> -    if (s->mastering_display_actual_peak_luminance_flag) {
>>> -        int rows, cols;
>>> -        if (get_bits_left(gb) < 10)
>>> -            return AVERROR_INVALIDDATA;
>>> -        rows = get_bits(gb, 5);
>>> -        cols = get_bits(gb, 5);
>>> -        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
>>> -            return AVERROR_INVALIDDATA;
>>> -        }
>>> -        s->num_rows_mastering_display_actual_peak_luminance = rows;
>>> -        s->num_cols_mastering_display_actual_peak_luminance = cols;
>>> -
>>> -        if (get_bits_left(gb) < (rows * cols * 4))
>>> -            return AVERROR_INVALIDDATA;
>>> -
>>> -        for (int i = 0; i < rows; i++) {
>>> -            for (int j = 0; j < cols; j++) {
>>> -                s->mastering_display_actual_peak_luminance[i][j] =
>>> -                    (AVRational){get_bits(gb, 4), peak_luminance_den};
>>> -            }
>>> -        }
>>> -    }
>>> -
>>> -    for (int w = 0; w < s->num_windows; w++) {
>>> -        AVHDRPlusColorTransformParams *params = &s->params[w];
>>> -        if (get_bits_left(gb) < 1)
>>> -            return AVERROR_INVALIDDATA;
>>> -
>>> -        params->tone_mapping_flag = get_bits1(gb);
>>> -        if (params->tone_mapping_flag) {
>>> -            if (get_bits_left(gb) < 28)
>>> -                return AVERROR_INVALIDDATA;
>>> -
>>> -            params->knee_point_x =
>>> -                (AVRational){get_bits(gb, 12), knee_point_den};
>>> -            params->knee_point_y =
>>> -                (AVRational){get_bits(gb, 12), knee_point_den};
>>> -            params->num_bezier_curve_anchors = get_bits(gb, 4);
>>> -
>>> -            if (get_bits_left(gb) < (params->num_bezier_curve_anchors * 10))
>>> -                return AVERROR_INVALIDDATA;
>>> -
>>> -            for (int i = 0; i < params->num_bezier_curve_anchors; i++) {
>>> -                params->bezier_curve_anchors[i] =
>>> -                    (AVRational){get_bits(gb, 10), bezier_anchor_den};
>>> -            }
>>> -        }
>>> -
>>> -        if (get_bits_left(gb) < 1)
>>> -            return AVERROR_INVALIDDATA;
>>> -        params->color_saturation_mapping_flag = get_bits1(gb);
>>> -        if (params->color_saturation_mapping_flag) {
>>> -            if (get_bits_left(gb) < 6)
>>> -                return AVERROR_INVALIDDATA;
>>> -            params->color_saturation_weight =
>>> -                (AVRational){get_bits(gb, 6), saturation_weight_den};
>>> -        }
>>> -    }
>>> -
>>> -    return 0;
>>> -}
>>> diff --git a/libavcodec/dynamic_hdr10_plus.h b/libavcodec/dynamic_hdr10_plus.h
>>> deleted file mode 100644
>>> index cd7acf0432..0000000000
>>> --- a/libavcodec/dynamic_hdr10_plus.h
>>> +++ /dev/null
>>> @@ -1,35 +0,0 @@
>>> -/*
>>> - * This file is part of FFmpeg.
>>> - *
>>> - * FFmpeg is free software; you can redistribute it and/or
>>> - * modify it under the terms of the GNU Lesser General Public
>>> - * License as published by the Free Software Foundation; either
>>> - * version 2.1 of the License, or (at your option) any later version.
>>> - *
>>> - * FFmpeg is distributed in the hope that it will be useful,
>>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> - * Lesser General Public License for more details.
>>> - *
>>> - * You should have received a copy of the GNU Lesser General Public
>>> - * License along with FFmpeg; if not, write to the Free Software
>>> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>>> - */
>>> -
>>> -#ifndef AVCODEC_DYNAMIC_HDR10_PLUS_H
>>> -#define AVCODEC_DYNAMIC_HDR10_PLUS_H
>>> -
>>> -#include "libavutil/hdr_dynamic_metadata.h"
>>> -
>>> -/**
>>> - * Parse the user data registered ITU-T T.35 to AVbuffer (AVDynamicHDRPlus).
>>> - * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
>>> - * @param data The byte array containing the raw ITU-T T.35 data.
>>> - * @param size Size of the data array in bytes.
>>> - *
>>> - * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
>>> - */
>>> -int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
>>> -                                             int size);
>>> -
>>> -#endif /* AVCODEC_DYNAMIC_HDR10_PLUS_H */
>>> diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
>>> index 6e4a9a1af2..63ab711bc9 100644
>>> --- a/libavcodec/h2645_sei.c
>>> +++ b/libavcodec/h2645_sei.c
>>> @@ -27,13 +27,13 @@
>>>
>>> #include "libavutil/ambient_viewing_environment.h"
>>> #include "libavutil/display.h"
>>> +#include "libavutil/hdr_dynamic_metadata.h"
>>> #include "libavutil/film_grain_params.h"
>>> #include "libavutil/pixdesc.h"
>>> #include "libavutil/stereo3d.h"
>>>
>>> #include "atsc_a53.h"
>>> #include "avcodec.h"
>>> -#include "dynamic_hdr10_plus.h"
>>> #include "dynamic_hdr_vivid.h"
>>> #include "get_bits.h"
>>> #include "golomb.h"
>>> @@ -52,8 +52,8 @@ static int decode_registered_user_data_dynamic_hdr_plus(HEVCSEIDynamicHDRPlus *s
>>>     if (!metadata)
>>>         return AVERROR(ENOMEM);
>>>
>>> -    err = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(metadata, gb->buffer,
>>> -                                                   bytestream2_get_bytes_left(gb));
>>> +    err = av_dynamic_hdr_plus_from_t35(metadata, gb->buffer,
>>> +                                       bytestream2_get_bytes_left(gb));
>>>     if (err < 0) {
>>>         av_free(metadata);
>>>         return err;
>>> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
>>> index eb1225ea1a..50c4ceee40 100644
>>> --- a/libavcodec/libdav1d.c
>>> +++ b/libavcodec/libdav1d.c
>>> @@ -24,6 +24,7 @@
>>> #include "libavutil/avassert.h"
>>> #include "libavutil/cpu.h"
>>> #include "libavutil/film_grain_params.h"
>>> +#include "libavutil/hdr_dynamic_metadata.h"
>>> #include "libavutil/mastering_display_metadata.h"
>>> #include "libavutil/imgutils.h"
>>> #include "libavutil/opt.h"
>>> @@ -33,7 +34,6 @@
>>> #include "bytestream.h"
>>> #include "codec_internal.h"
>>> #include "decode.h"
>>> -#include "dynamic_hdr10_plus.h"
>>> #include "internal.h"
>>>
>>> #define FF_DAV1D_VERSION_AT_LEAST(x,y) \
>>> @@ -555,8 +555,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>>                 goto fail;
>>>             }
>>>
>>> -            res = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer,
>>> -                                                           bytestream2_get_bytes_left(&gb));
>>> +            res = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer,
>>> +                                               bytestream2_get_bytes_left(&gb));
>>>             if (res < 0)
>>>                 goto fail;
>>>             break;
>>> diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
>>> index 0fa1ee82de..5ed903f475 100644
>>> --- a/libavutil/hdr_dynamic_metadata.c
>>> +++ b/libavutil/hdr_dynamic_metadata.c
>>> @@ -20,6 +20,18 @@
>>>
>>> #include "hdr_dynamic_metadata.h"
>>> #include "mem.h"
>>> +#include "libavcodec/get_bits.h"
>>> +#include "libavcodec/put_bits.h"
>>> +
>>> +#define T35_PAYLOAD_MAX_SIZE 907
>>> +
>>> +static const int64_t luminance_den = 1;
>>> +static const int32_t peak_luminance_den = 15;
>>> +static const int64_t rgb_den = 100000;
>>> +static const int32_t fraction_pixel_den = 1000;
>>> +static const int32_t knee_point_den = 4095;
>>> +static const int32_t bezier_anchor_den = 1023;
>>> +static const int32_t saturation_weight_den = 8;
>>>
>>> AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size)
>>> {
>>> @@ -45,3 +57,176 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame)
>>>
>>>     return (AVDynamicHDRPlus *)side_data->data;
>>> }
>>> +
>>> +int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>>> +                                 size_t size)
>>> +{
>>> +    uint8_t padded_buf[T35_PAYLOAD_MAX_SIZE + AV_INPUT_BUFFER_PADDING_SIZE];
>>> +    GetBitContext gbc, *gb = &gbc;
>>> +    int ret;
>>> +
>>> +    if (!s)
>>> +        return AVERROR(ENOMEM);
>>> +
>>> +    memcpy(padded_buf, data, size);
>>
>> You are potentially copying more than T35_PAYLOAD_MAX_SIZE bytes.
>> Furthermore, you are not zeroing the padding; this might make Valgrind
>> complain.
> 
> Good point on the size if the user sends an incorrect value, we can return AVERROR(EINVAL) in that case. I did run it through Valgrind and did not get an error due to not zeroing the padded buffer. I don’t think that should be necessary since it’s contained within that function and uninitialized bits should never be accessed, is that incorrect?
> 

The GetBit API can overread a bit. It mostly reads 32bits at a time, so
that it can read into the padding even when no bit of the padding is
actually used (because it is masked away); but Valgrind does not always
get this.

- Andreas
Raphaël Zumer March 16, 2023, 9:27 p.m. UTC | #4
On 3/16/23 16:50, Andreas Rheinhardt wrote:
> Raphaël Zumer:
>>
>>> On Mar 16, 2023, at 16:19, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
>>>
>>> Raphaël Zumer:
>>>> Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
>>>> ---
>>>> libavcodec/Makefile              |   6 +-
>>>> libavcodec/av1dec.c              |   6 +-
>>>> libavcodec/dynamic_hdr10_plus.c  | 198 -------------------------------
>>>> libavcodec/dynamic_hdr10_plus.h  |  35 ------
>>>> libavcodec/h2645_sei.c           |   6 +-
>>>> libavcodec/libdav1d.c            |   6 +-
>>>> libavutil/hdr_dynamic_metadata.c | 185 +++++++++++++++++++++++++++++
>>>> libavutil/hdr_dynamic_metadata.h |  13 ++
>>>> 8 files changed, 210 insertions(+), 245 deletions(-)
>>>> delete mode 100644 libavcodec/dynamic_hdr10_plus.c
>>>> delete mode 100644 libavcodec/dynamic_hdr10_plus.h
>>>>
>>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>>>> index abae4909d2..408ecd1e31 100644
>>>> --- a/libavcodec/Makefile
>>>> +++ b/libavcodec/Makefile
>>>> @@ -104,7 +104,7 @@ OBJS-$(CONFIG_H264_SEI)                += h264_sei.o h2645_sei.o
>>>> OBJS-$(CONFIG_HEVCPARSE)               += hevc_parse.o hevc_ps.o hevc_data.o \
>>>>                                           h2645data.o h2645_parse.o h2645_vui.o
>>>> OBJS-$(CONFIG_HEVC_SEI)                += hevc_sei.o h2645_sei.o \
>>>> -                                          dynamic_hdr10_plus.o dynamic_hdr_vivid.o
>>>> +                                          dynamic_hdr_vivid.o
>>>> OBJS-$(CONFIG_HPELDSP)                 += hpeldsp.o
>>>> OBJS-$(CONFIG_HUFFMAN)                 += huffman.o
>>>> OBJS-$(CONFIG_HUFFYUVDSP)              += huffyuvdsp.o
>>>> @@ -250,7 +250,7 @@ OBJS-$(CONFIG_ATRAC3PAL_DECODER)       += atrac3plusdec.o atrac3plus.o \
>>>> OBJS-$(CONFIG_ATRAC9_DECODER)          += atrac9dec.o
>>>> OBJS-$(CONFIG_AURA_DECODER)            += cyuv.o
>>>> OBJS-$(CONFIG_AURA2_DECODER)           += aura.o
>>>> -OBJS-$(CONFIG_AV1_DECODER)             += av1dec.o dynamic_hdr10_plus.o
>>>> +OBJS-$(CONFIG_AV1_DECODER)             += av1dec.o
>>>> OBJS-$(CONFIG_AV1_CUVID_DECODER)       += cuviddec.o
>>>> OBJS-$(CONFIG_AV1_MEDIACODEC_DECODER)  += mediacodecdec.o
>>>> OBJS-$(CONFIG_AV1_NVENC_ENCODER)       += nvenc_av1.o nvenc.o
>>>> @@ -1082,7 +1082,7 @@ OBJS-$(CONFIG_LIBARIBB24_DECODER)         += libaribb24.o ass.o
>>>> OBJS-$(CONFIG_LIBCELT_DECODER)            += libcelt_dec.o
>>>> OBJS-$(CONFIG_LIBCODEC2_DECODER)          += libcodec2.o
>>>> OBJS-$(CONFIG_LIBCODEC2_ENCODER)          += libcodec2.o
>>>> -OBJS-$(CONFIG_LIBDAV1D_DECODER)           += libdav1d.o dynamic_hdr10_plus.o
>>>> +OBJS-$(CONFIG_LIBDAV1D_DECODER)           += libdav1d.o
>>>> OBJS-$(CONFIG_LIBDAVS2_DECODER)           += libdavs2.o
>>>> OBJS-$(CONFIG_LIBFDK_AAC_DECODER)         += libfdk-aacdec.o
>>>> OBJS-$(CONFIG_LIBFDK_AAC_ENCODER)         += libfdk-aacenc.o
>>>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>>>> index a80e37e33f..df393fe3d0 100644
>>>> --- a/libavcodec/av1dec.c
>>>> +++ b/libavcodec/av1dec.c
>>>> @@ -20,6 +20,7 @@
>>>>
>>>> #include "config_components.h"
>>>>
>>>> +#include "libavutil/hdr_dynamic_metadata.h"
>>>> #include "libavutil/film_grain_params.h"
>>>> #include "libavutil/mastering_display_metadata.h"
>>>> #include "libavutil/pixdesc.h"
>>>> @@ -30,7 +31,6 @@
>>>> #include "bytestream.h"
>>>> #include "codec_internal.h"
>>>> #include "decode.h"
>>>> -#include "dynamic_hdr10_plus.h"
>>>> #include "hwconfig.h"
>>>> #include "profiles.h"
>>>> #include "thread.h"
>>>> @@ -925,8 +925,8 @@ static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame,
>>>>         if (!hdrplus)
>>>>             return AVERROR(ENOMEM);
>>>>
>>>> -        ret = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer,
>>>> -                                                       bytestream2_get_bytes_left(&gb));
>>>> +        ret = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer,
>>>> +                                           bytestream2_get_bytes_left(&gb));
>>>>         if (ret < 0)
>>>>             return ret;
>>>>         break;
>>>> diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c
>>>> deleted file mode 100644
>>>> index 34a44aac65..0000000000
>>>> --- a/libavcodec/dynamic_hdr10_plus.c
>>>> +++ /dev/null
>>>> @@ -1,198 +0,0 @@
>>>> -/*
>>>> - * This file is part of FFmpeg.
>>>> - *
>>>> - * FFmpeg is free software; you can redistribute it and/or
>>>> - * modify it under the terms of the GNU Lesser General Public
>>>> - * License as published by the Free Software Foundation; either
>>>> - * version 2.1 of the License, or (at your option) any later version.
>>>> - *
>>>> - * FFmpeg is distributed in the hope that it will be useful,
>>>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>> - * Lesser General Public License for more details.
>>>> - *
>>>> - * You should have received a copy of the GNU Lesser General Public
>>>> - * License along with FFmpeg; if not, write to the Free Software
>>>> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>>>> - */
>>>> -
>>>> -#include "dynamic_hdr10_plus.h"
>>>> -#include "get_bits.h"
>>>> -
>>>> -static const int64_t luminance_den = 1;
>>>> -static const int32_t peak_luminance_den = 15;
>>>> -static const int64_t rgb_den = 100000;
>>>> -static const int32_t fraction_pixel_den = 1000;
>>>> -static const int32_t knee_point_den = 4095;
>>>> -static const int32_t bezier_anchor_den = 1023;
>>>> -static const int32_t saturation_weight_den = 8;
>>>> -
>>>> -int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
>>>> -                                             int size)
>>>> -{
>>>> -    GetBitContext gbc, *gb = &gbc;
>>>> -    int ret;
>>>> -
>>>> -    if (!s)
>>>> -        return AVERROR(ENOMEM);
>>>> -
>>>> -    ret = init_get_bits8(gb, data, size);
>>>> -    if (ret < 0)
>>>> -        return ret;
>>>> -
>>>> -    if (get_bits_left(gb) < 10)
>>>> -        return AVERROR_INVALIDDATA;
>>>> -
>>>> -    s->application_version = get_bits(gb, 8);
>>>> -    s->num_windows = get_bits(gb, 2);
>>>> -
>>>> -    if (s->num_windows < 1 || s->num_windows > 3) {
>>>> -        return AVERROR_INVALIDDATA;
>>>> -    }
>>>> -
>>>> -    if (get_bits_left(gb) < ((19 * 8 + 1) * (s->num_windows - 1)))
>>>> -        return AVERROR_INVALIDDATA;
>>>> -
>>>> -    for (int w = 1; w < s->num_windows; w++) {
>>>> -        // The corners are set to absolute coordinates here. They should be
>>>> -        // converted to the relative coordinates (in [0, 1]) in the decoder.
>>>> -        AVHDRPlusColorTransformParams *params = &s->params[w];
>>>> -        params->window_upper_left_corner_x =
>>>> -            (AVRational){get_bits(gb, 16), 1};
>>>> -        params->window_upper_left_corner_y =
>>>> -            (AVRational){get_bits(gb, 16), 1};
>>>> -        params->window_lower_right_corner_x =
>>>> -            (AVRational){get_bits(gb, 16), 1};
>>>> -        params->window_lower_right_corner_y =
>>>> -            (AVRational){get_bits(gb, 16), 1};
>>>> -
>>>> -        params->center_of_ellipse_x = get_bits(gb, 16);
>>>> -        params->center_of_ellipse_y = get_bits(gb, 16);
>>>> -        params->rotation_angle = get_bits(gb, 8);
>>>> -        params->semimajor_axis_internal_ellipse = get_bits(gb, 16);
>>>> -        params->semimajor_axis_external_ellipse = get_bits(gb, 16);
>>>> -        params->semiminor_axis_external_ellipse = get_bits(gb, 16);
>>>> -        params->overlap_process_option = get_bits1(gb);
>>>> -    }
>>>> -
>>>> -    if (get_bits_left(gb) < 28)
>>>> -        return AVERROR_INVALIDDATA;
>>>> -
>>>> -    s->targeted_system_display_maximum_luminance =
>>>> -        (AVRational){get_bits_long(gb, 27), luminance_den};
>>>> -    s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
>>>> -
>>>> -    if (s->targeted_system_display_actual_peak_luminance_flag) {
>>>> -        int rows, cols;
>>>> -        if (get_bits_left(gb) < 10)
>>>> -            return AVERROR_INVALIDDATA;
>>>> -        rows = get_bits(gb, 5);
>>>> -        cols = get_bits(gb, 5);
>>>> -        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
>>>> -            return AVERROR_INVALIDDATA;
>>>> -        }
>>>> -        s->num_rows_targeted_system_display_actual_peak_luminance = rows;
>>>> -        s->num_cols_targeted_system_display_actual_peak_luminance = cols;
>>>> -
>>>> -        if (get_bits_left(gb) < (rows * cols * 4))
>>>> -            return AVERROR_INVALIDDATA;
>>>> -
>>>> -        for (int i = 0; i < rows; i++) {
>>>> -            for (int j = 0; j < cols; j++) {
>>>> -                s->targeted_system_display_actual_peak_luminance[i][j] =
>>>> -                    (AVRational){get_bits(gb, 4), peak_luminance_den};
>>>> -            }
>>>> -        }
>>>> -    }
>>>> -    for (int w = 0; w < s->num_windows; w++) {
>>>> -        AVHDRPlusColorTransformParams *params = &s->params[w];
>>>> -        if (get_bits_left(gb) < (3 * 17 + 17 + 4))
>>>> -            return AVERROR_INVALIDDATA;
>>>> -
>>>> -        for (int i = 0; i < 3; i++) {
>>>> -            params->maxscl[i] =
>>>> -                (AVRational){get_bits(gb, 17), rgb_den};
>>>> -        }
>>>> -        params->average_maxrgb =
>>>> -            (AVRational){get_bits(gb, 17), rgb_den};
>>>> -        params->num_distribution_maxrgb_percentiles = get_bits(gb, 4);
>>>> -
>>>> -        if (get_bits_left(gb) <
>>>> -            (params->num_distribution_maxrgb_percentiles * 24))
>>>> -            return AVERROR_INVALIDDATA;
>>>> -
>>>> -        for (int i = 0; i < params->num_distribution_maxrgb_percentiles; i++) {
>>>> -            params->distribution_maxrgb[i].percentage = get_bits(gb, 7);
>>>> -            params->distribution_maxrgb[i].percentile =
>>>> -                (AVRational){get_bits(gb, 17), rgb_den};
>>>> -        }
>>>> -
>>>> -        if (get_bits_left(gb) < 10)
>>>> -            return AVERROR_INVALIDDATA;
>>>> -
>>>> -        params->fraction_bright_pixels = (AVRational){get_bits(gb, 10), fraction_pixel_den};
>>>> -    }
>>>> -    if (get_bits_left(gb) < 1)
>>>> -        return AVERROR_INVALIDDATA;
>>>> -    s->mastering_display_actual_peak_luminance_flag = get_bits1(gb);
>>>> -    if (s->mastering_display_actual_peak_luminance_flag) {
>>>> -        int rows, cols;
>>>> -        if (get_bits_left(gb) < 10)
>>>> -            return AVERROR_INVALIDDATA;
>>>> -        rows = get_bits(gb, 5);
>>>> -        cols = get_bits(gb, 5);
>>>> -        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
>>>> -            return AVERROR_INVALIDDATA;
>>>> -        }
>>>> -        s->num_rows_mastering_display_actual_peak_luminance = rows;
>>>> -        s->num_cols_mastering_display_actual_peak_luminance = cols;
>>>> -
>>>> -        if (get_bits_left(gb) < (rows * cols * 4))
>>>> -            return AVERROR_INVALIDDATA;
>>>> -
>>>> -        for (int i = 0; i < rows; i++) {
>>>> -            for (int j = 0; j < cols; j++) {
>>>> -                s->mastering_display_actual_peak_luminance[i][j] =
>>>> -                    (AVRational){get_bits(gb, 4), peak_luminance_den};
>>>> -            }
>>>> -        }
>>>> -    }
>>>> -
>>>> -    for (int w = 0; w < s->num_windows; w++) {
>>>> -        AVHDRPlusColorTransformParams *params = &s->params[w];
>>>> -        if (get_bits_left(gb) < 1)
>>>> -            return AVERROR_INVALIDDATA;
>>>> -
>>>> -        params->tone_mapping_flag = get_bits1(gb);
>>>> -        if (params->tone_mapping_flag) {
>>>> -            if (get_bits_left(gb) < 28)
>>>> -                return AVERROR_INVALIDDATA;
>>>> -
>>>> -            params->knee_point_x =
>>>> -                (AVRational){get_bits(gb, 12), knee_point_den};
>>>> -            params->knee_point_y =
>>>> -                (AVRational){get_bits(gb, 12), knee_point_den};
>>>> -            params->num_bezier_curve_anchors = get_bits(gb, 4);
>>>> -
>>>> -            if (get_bits_left(gb) < (params->num_bezier_curve_anchors * 10))
>>>> -                return AVERROR_INVALIDDATA;
>>>> -
>>>> -            for (int i = 0; i < params->num_bezier_curve_anchors; i++) {
>>>> -                params->bezier_curve_anchors[i] =
>>>> -                    (AVRational){get_bits(gb, 10), bezier_anchor_den};
>>>> -            }
>>>> -        }
>>>> -
>>>> -        if (get_bits_left(gb) < 1)
>>>> -            return AVERROR_INVALIDDATA;
>>>> -        params->color_saturation_mapping_flag = get_bits1(gb);
>>>> -        if (params->color_saturation_mapping_flag) {
>>>> -            if (get_bits_left(gb) < 6)
>>>> -                return AVERROR_INVALIDDATA;
>>>> -            params->color_saturation_weight =
>>>> -                (AVRational){get_bits(gb, 6), saturation_weight_den};
>>>> -        }
>>>> -    }
>>>> -
>>>> -    return 0;
>>>> -}
>>>> diff --git a/libavcodec/dynamic_hdr10_plus.h b/libavcodec/dynamic_hdr10_plus.h
>>>> deleted file mode 100644
>>>> index cd7acf0432..0000000000
>>>> --- a/libavcodec/dynamic_hdr10_plus.h
>>>> +++ /dev/null
>>>> @@ -1,35 +0,0 @@
>>>> -/*
>>>> - * This file is part of FFmpeg.
>>>> - *
>>>> - * FFmpeg is free software; you can redistribute it and/or
>>>> - * modify it under the terms of the GNU Lesser General Public
>>>> - * License as published by the Free Software Foundation; either
>>>> - * version 2.1 of the License, or (at your option) any later version.
>>>> - *
>>>> - * FFmpeg is distributed in the hope that it will be useful,
>>>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>> - * Lesser General Public License for more details.
>>>> - *
>>>> - * You should have received a copy of the GNU Lesser General Public
>>>> - * License along with FFmpeg; if not, write to the Free Software
>>>> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>>>> - */
>>>> -
>>>> -#ifndef AVCODEC_DYNAMIC_HDR10_PLUS_H
>>>> -#define AVCODEC_DYNAMIC_HDR10_PLUS_H
>>>> -
>>>> -#include "libavutil/hdr_dynamic_metadata.h"
>>>> -
>>>> -/**
>>>> - * Parse the user data registered ITU-T T.35 to AVbuffer (AVDynamicHDRPlus).
>>>> - * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
>>>> - * @param data The byte array containing the raw ITU-T T.35 data.
>>>> - * @param size Size of the data array in bytes.
>>>> - *
>>>> - * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
>>>> - */
>>>> -int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
>>>> -                                             int size);
>>>> -
>>>> -#endif /* AVCODEC_DYNAMIC_HDR10_PLUS_H */
>>>> diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
>>>> index 6e4a9a1af2..63ab711bc9 100644
>>>> --- a/libavcodec/h2645_sei.c
>>>> +++ b/libavcodec/h2645_sei.c
>>>> @@ -27,13 +27,13 @@
>>>>
>>>> #include "libavutil/ambient_viewing_environment.h"
>>>> #include "libavutil/display.h"
>>>> +#include "libavutil/hdr_dynamic_metadata.h"
>>>> #include "libavutil/film_grain_params.h"
>>>> #include "libavutil/pixdesc.h"
>>>> #include "libavutil/stereo3d.h"
>>>>
>>>> #include "atsc_a53.h"
>>>> #include "avcodec.h"
>>>> -#include "dynamic_hdr10_plus.h"
>>>> #include "dynamic_hdr_vivid.h"
>>>> #include "get_bits.h"
>>>> #include "golomb.h"
>>>> @@ -52,8 +52,8 @@ static int decode_registered_user_data_dynamic_hdr_plus(HEVCSEIDynamicHDRPlus *s
>>>>     if (!metadata)
>>>>         return AVERROR(ENOMEM);
>>>>
>>>> -    err = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(metadata, gb->buffer,
>>>> -                                                   bytestream2_get_bytes_left(gb));
>>>> +    err = av_dynamic_hdr_plus_from_t35(metadata, gb->buffer,
>>>> +                                       bytestream2_get_bytes_left(gb));
>>>>     if (err < 0) {
>>>>         av_free(metadata);
>>>>         return err;
>>>> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
>>>> index eb1225ea1a..50c4ceee40 100644
>>>> --- a/libavcodec/libdav1d.c
>>>> +++ b/libavcodec/libdav1d.c
>>>> @@ -24,6 +24,7 @@
>>>> #include "libavutil/avassert.h"
>>>> #include "libavutil/cpu.h"
>>>> #include "libavutil/film_grain_params.h"
>>>> +#include "libavutil/hdr_dynamic_metadata.h"
>>>> #include "libavutil/mastering_display_metadata.h"
>>>> #include "libavutil/imgutils.h"
>>>> #include "libavutil/opt.h"
>>>> @@ -33,7 +34,6 @@
>>>> #include "bytestream.h"
>>>> #include "codec_internal.h"
>>>> #include "decode.h"
>>>> -#include "dynamic_hdr10_plus.h"
>>>> #include "internal.h"
>>>>
>>>> #define FF_DAV1D_VERSION_AT_LEAST(x,y) \
>>>> @@ -555,8 +555,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>>>                 goto fail;
>>>>             }
>>>>
>>>> -            res = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer,
>>>> -                                                           bytestream2_get_bytes_left(&gb));
>>>> +            res = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer,
>>>> +                                               bytestream2_get_bytes_left(&gb));
>>>>             if (res < 0)
>>>>                 goto fail;
>>>>             break;
>>>> diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
>>>> index 0fa1ee82de..5ed903f475 100644
>>>> --- a/libavutil/hdr_dynamic_metadata.c
>>>> +++ b/libavutil/hdr_dynamic_metadata.c
>>>> @@ -20,6 +20,18 @@
>>>>
>>>> #include "hdr_dynamic_metadata.h"
>>>> #include "mem.h"
>>>> +#include "libavcodec/get_bits.h"
>>>> +#include "libavcodec/put_bits.h"
>>>> +
>>>> +#define T35_PAYLOAD_MAX_SIZE 907
>>>> +
>>>> +static const int64_t luminance_den = 1;
>>>> +static const int32_t peak_luminance_den = 15;
>>>> +static const int64_t rgb_den = 100000;
>>>> +static const int32_t fraction_pixel_den = 1000;
>>>> +static const int32_t knee_point_den = 4095;
>>>> +static const int32_t bezier_anchor_den = 1023;
>>>> +static const int32_t saturation_weight_den = 8;
>>>>
>>>> AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size)
>>>> {
>>>> @@ -45,3 +57,176 @@ AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame)
>>>>
>>>>     return (AVDynamicHDRPlus *)side_data->data;
>>>> }
>>>> +
>>>> +int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
>>>> +                                 size_t size)
>>>> +{
>>>> +    uint8_t padded_buf[T35_PAYLOAD_MAX_SIZE + AV_INPUT_BUFFER_PADDING_SIZE];
>>>> +    GetBitContext gbc, *gb = &gbc;
>>>> +    int ret;
>>>> +
>>>> +    if (!s)
>>>> +        return AVERROR(ENOMEM);
>>>> +
>>>> +    memcpy(padded_buf, data, size);
>>> You are potentially copying more than T35_PAYLOAD_MAX_SIZE bytes.
>>> Furthermore, you are not zeroing the padding; this might make Valgrind
>>> complain.
>> Good point on the size if the user sends an incorrect value, we can return AVERROR(EINVAL) in that case. I did run it through Valgrind and did not get an error due to not zeroing the padded buffer. I don’t think that should be necessary since it’s contained within that function and uninitialized bits should never be accessed, is that incorrect?
>>
> The GetBit API can overread a bit. It mostly reads 32bits at a time, so
> that it can read into the padding even when no bit of the padding is
> actually used (because it is masked away); but Valgrind does not always
> get this.
>
> - Andreas

OK; I resent the first patch as v8 with these changes.

RZ
diff mbox series

Patch

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index abae4909d2..408ecd1e31 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -104,7 +104,7 @@  OBJS-$(CONFIG_H264_SEI)                += h264_sei.o h2645_sei.o
 OBJS-$(CONFIG_HEVCPARSE)               += hevc_parse.o hevc_ps.o hevc_data.o \
                                           h2645data.o h2645_parse.o h2645_vui.o
 OBJS-$(CONFIG_HEVC_SEI)                += hevc_sei.o h2645_sei.o \
-                                          dynamic_hdr10_plus.o dynamic_hdr_vivid.o
+                                          dynamic_hdr_vivid.o
 OBJS-$(CONFIG_HPELDSP)                 += hpeldsp.o
 OBJS-$(CONFIG_HUFFMAN)                 += huffman.o
 OBJS-$(CONFIG_HUFFYUVDSP)              += huffyuvdsp.o
@@ -250,7 +250,7 @@  OBJS-$(CONFIG_ATRAC3PAL_DECODER)       += atrac3plusdec.o atrac3plus.o \
 OBJS-$(CONFIG_ATRAC9_DECODER)          += atrac9dec.o
 OBJS-$(CONFIG_AURA_DECODER)            += cyuv.o
 OBJS-$(CONFIG_AURA2_DECODER)           += aura.o
-OBJS-$(CONFIG_AV1_DECODER)             += av1dec.o dynamic_hdr10_plus.o
+OBJS-$(CONFIG_AV1_DECODER)             += av1dec.o
 OBJS-$(CONFIG_AV1_CUVID_DECODER)       += cuviddec.o
 OBJS-$(CONFIG_AV1_MEDIACODEC_DECODER)  += mediacodecdec.o
 OBJS-$(CONFIG_AV1_NVENC_ENCODER)       += nvenc_av1.o nvenc.o
@@ -1082,7 +1082,7 @@  OBJS-$(CONFIG_LIBARIBB24_DECODER)         += libaribb24.o ass.o
 OBJS-$(CONFIG_LIBCELT_DECODER)            += libcelt_dec.o
 OBJS-$(CONFIG_LIBCODEC2_DECODER)          += libcodec2.o
 OBJS-$(CONFIG_LIBCODEC2_ENCODER)          += libcodec2.o
-OBJS-$(CONFIG_LIBDAV1D_DECODER)           += libdav1d.o dynamic_hdr10_plus.o
+OBJS-$(CONFIG_LIBDAV1D_DECODER)           += libdav1d.o
 OBJS-$(CONFIG_LIBDAVS2_DECODER)           += libdavs2.o
 OBJS-$(CONFIG_LIBFDK_AAC_DECODER)         += libfdk-aacdec.o
 OBJS-$(CONFIG_LIBFDK_AAC_ENCODER)         += libfdk-aacenc.o
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index a80e37e33f..df393fe3d0 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -20,6 +20,7 @@ 
 
 #include "config_components.h"
 
+#include "libavutil/hdr_dynamic_metadata.h"
 #include "libavutil/film_grain_params.h"
 #include "libavutil/mastering_display_metadata.h"
 #include "libavutil/pixdesc.h"
@@ -30,7 +31,6 @@ 
 #include "bytestream.h"
 #include "codec_internal.h"
 #include "decode.h"
-#include "dynamic_hdr10_plus.h"
 #include "hwconfig.h"
 #include "profiles.h"
 #include "thread.h"
@@ -925,8 +925,8 @@  static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame,
         if (!hdrplus)
             return AVERROR(ENOMEM);
 
-        ret = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer,
-                                                       bytestream2_get_bytes_left(&gb));
+        ret = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer,
+                                           bytestream2_get_bytes_left(&gb));
         if (ret < 0)
             return ret;
         break;
diff --git a/libavcodec/dynamic_hdr10_plus.c b/libavcodec/dynamic_hdr10_plus.c
deleted file mode 100644
index 34a44aac65..0000000000
--- a/libavcodec/dynamic_hdr10_plus.c
+++ /dev/null
@@ -1,198 +0,0 @@ 
-/*
- * This file is part of FFmpeg.
- *
- * FFmpeg is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * FFmpeg is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with FFmpeg; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-#include "dynamic_hdr10_plus.h"
-#include "get_bits.h"
-
-static const int64_t luminance_den = 1;
-static const int32_t peak_luminance_den = 15;
-static const int64_t rgb_den = 100000;
-static const int32_t fraction_pixel_den = 1000;
-static const int32_t knee_point_den = 4095;
-static const int32_t bezier_anchor_den = 1023;
-static const int32_t saturation_weight_den = 8;
-
-int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
-                                             int size)
-{
-    GetBitContext gbc, *gb = &gbc;
-    int ret;
-
-    if (!s)
-        return AVERROR(ENOMEM);
-
-    ret = init_get_bits8(gb, data, size);
-    if (ret < 0)
-        return ret;
-
-    if (get_bits_left(gb) < 10)
-        return AVERROR_INVALIDDATA;
-
-    s->application_version = get_bits(gb, 8);
-    s->num_windows = get_bits(gb, 2);
-
-    if (s->num_windows < 1 || s->num_windows > 3) {
-        return AVERROR_INVALIDDATA;
-    }
-
-    if (get_bits_left(gb) < ((19 * 8 + 1) * (s->num_windows - 1)))
-        return AVERROR_INVALIDDATA;
-
-    for (int w = 1; w < s->num_windows; w++) {
-        // The corners are set to absolute coordinates here. They should be
-        // converted to the relative coordinates (in [0, 1]) in the decoder.
-        AVHDRPlusColorTransformParams *params = &s->params[w];
-        params->window_upper_left_corner_x =
-            (AVRational){get_bits(gb, 16), 1};
-        params->window_upper_left_corner_y =
-            (AVRational){get_bits(gb, 16), 1};
-        params->window_lower_right_corner_x =
-            (AVRational){get_bits(gb, 16), 1};
-        params->window_lower_right_corner_y =
-            (AVRational){get_bits(gb, 16), 1};
-
-        params->center_of_ellipse_x = get_bits(gb, 16);
-        params->center_of_ellipse_y = get_bits(gb, 16);
-        params->rotation_angle = get_bits(gb, 8);
-        params->semimajor_axis_internal_ellipse = get_bits(gb, 16);
-        params->semimajor_axis_external_ellipse = get_bits(gb, 16);
-        params->semiminor_axis_external_ellipse = get_bits(gb, 16);
-        params->overlap_process_option = get_bits1(gb);
-    }
-
-    if (get_bits_left(gb) < 28)
-        return AVERROR_INVALIDDATA;
-
-    s->targeted_system_display_maximum_luminance =
-        (AVRational){get_bits_long(gb, 27), luminance_den};
-    s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
-
-    if (s->targeted_system_display_actual_peak_luminance_flag) {
-        int rows, cols;
-        if (get_bits_left(gb) < 10)
-            return AVERROR_INVALIDDATA;
-        rows = get_bits(gb, 5);
-        cols = get_bits(gb, 5);
-        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
-            return AVERROR_INVALIDDATA;
-        }
-        s->num_rows_targeted_system_display_actual_peak_luminance = rows;
-        s->num_cols_targeted_system_display_actual_peak_luminance = cols;
-
-        if (get_bits_left(gb) < (rows * cols * 4))
-            return AVERROR_INVALIDDATA;
-
-        for (int i = 0; i < rows; i++) {
-            for (int j = 0; j < cols; j++) {
-                s->targeted_system_display_actual_peak_luminance[i][j] =
-                    (AVRational){get_bits(gb, 4), peak_luminance_den};
-            }
-        }
-    }
-    for (int w = 0; w < s->num_windows; w++) {
-        AVHDRPlusColorTransformParams *params = &s->params[w];
-        if (get_bits_left(gb) < (3 * 17 + 17 + 4))
-            return AVERROR_INVALIDDATA;
-
-        for (int i = 0; i < 3; i++) {
-            params->maxscl[i] =
-                (AVRational){get_bits(gb, 17), rgb_den};
-        }
-        params->average_maxrgb =
-            (AVRational){get_bits(gb, 17), rgb_den};
-        params->num_distribution_maxrgb_percentiles = get_bits(gb, 4);
-
-        if (get_bits_left(gb) <
-            (params->num_distribution_maxrgb_percentiles * 24))
-            return AVERROR_INVALIDDATA;
-
-        for (int i = 0; i < params->num_distribution_maxrgb_percentiles; i++) {
-            params->distribution_maxrgb[i].percentage = get_bits(gb, 7);
-            params->distribution_maxrgb[i].percentile =
-                (AVRational){get_bits(gb, 17), rgb_den};
-        }
-
-        if (get_bits_left(gb) < 10)
-            return AVERROR_INVALIDDATA;
-
-        params->fraction_bright_pixels = (AVRational){get_bits(gb, 10), fraction_pixel_den};
-    }
-    if (get_bits_left(gb) < 1)
-        return AVERROR_INVALIDDATA;
-    s->mastering_display_actual_peak_luminance_flag = get_bits1(gb);
-    if (s->mastering_display_actual_peak_luminance_flag) {
-        int rows, cols;
-        if (get_bits_left(gb) < 10)
-            return AVERROR_INVALIDDATA;
-        rows = get_bits(gb, 5);
-        cols = get_bits(gb, 5);
-        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
-            return AVERROR_INVALIDDATA;
-        }
-        s->num_rows_mastering_display_actual_peak_luminance = rows;
-        s->num_cols_mastering_display_actual_peak_luminance = cols;
-
-        if (get_bits_left(gb) < (rows * cols * 4))
-            return AVERROR_INVALIDDATA;
-
-        for (int i = 0; i < rows; i++) {
-            for (int j = 0; j < cols; j++) {
-                s->mastering_display_actual_peak_luminance[i][j] =
-                    (AVRational){get_bits(gb, 4), peak_luminance_den};
-            }
-        }
-    }
-
-    for (int w = 0; w < s->num_windows; w++) {
-        AVHDRPlusColorTransformParams *params = &s->params[w];
-        if (get_bits_left(gb) < 1)
-            return AVERROR_INVALIDDATA;
-
-        params->tone_mapping_flag = get_bits1(gb);
-        if (params->tone_mapping_flag) {
-            if (get_bits_left(gb) < 28)
-                return AVERROR_INVALIDDATA;
-
-            params->knee_point_x =
-                (AVRational){get_bits(gb, 12), knee_point_den};
-            params->knee_point_y =
-                (AVRational){get_bits(gb, 12), knee_point_den};
-            params->num_bezier_curve_anchors = get_bits(gb, 4);
-
-            if (get_bits_left(gb) < (params->num_bezier_curve_anchors * 10))
-                return AVERROR_INVALIDDATA;
-
-            for (int i = 0; i < params->num_bezier_curve_anchors; i++) {
-                params->bezier_curve_anchors[i] =
-                    (AVRational){get_bits(gb, 10), bezier_anchor_den};
-            }
-        }
-
-        if (get_bits_left(gb) < 1)
-            return AVERROR_INVALIDDATA;
-        params->color_saturation_mapping_flag = get_bits1(gb);
-        if (params->color_saturation_mapping_flag) {
-            if (get_bits_left(gb) < 6)
-                return AVERROR_INVALIDDATA;
-            params->color_saturation_weight =
-                (AVRational){get_bits(gb, 6), saturation_weight_den};
-        }
-    }
-
-    return 0;
-}
diff --git a/libavcodec/dynamic_hdr10_plus.h b/libavcodec/dynamic_hdr10_plus.h
deleted file mode 100644
index cd7acf0432..0000000000
--- a/libavcodec/dynamic_hdr10_plus.h
+++ /dev/null
@@ -1,35 +0,0 @@ 
-/*
- * This file is part of FFmpeg.
- *
- * FFmpeg is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * FFmpeg is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with FFmpeg; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-#ifndef AVCODEC_DYNAMIC_HDR10_PLUS_H
-#define AVCODEC_DYNAMIC_HDR10_PLUS_H
-
-#include "libavutil/hdr_dynamic_metadata.h"
-
-/**
- * Parse the user data registered ITU-T T.35 to AVbuffer (AVDynamicHDRPlus).
- * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
- * @param data The byte array containing the raw ITU-T T.35 data.
- * @param size Size of the data array in bytes.
- *
- * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
- */
-int ff_parse_itu_t_t35_to_dynamic_hdr10_plus(AVDynamicHDRPlus *s, const uint8_t *data,
-                                             int size);
-
-#endif /* AVCODEC_DYNAMIC_HDR10_PLUS_H */
diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
index 6e4a9a1af2..63ab711bc9 100644
--- a/libavcodec/h2645_sei.c
+++ b/libavcodec/h2645_sei.c
@@ -27,13 +27,13 @@ 
 
 #include "libavutil/ambient_viewing_environment.h"
 #include "libavutil/display.h"
+#include "libavutil/hdr_dynamic_metadata.h"
 #include "libavutil/film_grain_params.h"
 #include "libavutil/pixdesc.h"
 #include "libavutil/stereo3d.h"
 
 #include "atsc_a53.h"
 #include "avcodec.h"
-#include "dynamic_hdr10_plus.h"
 #include "dynamic_hdr_vivid.h"
 #include "get_bits.h"
 #include "golomb.h"
@@ -52,8 +52,8 @@  static int decode_registered_user_data_dynamic_hdr_plus(HEVCSEIDynamicHDRPlus *s
     if (!metadata)
         return AVERROR(ENOMEM);
 
-    err = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(metadata, gb->buffer,
-                                                   bytestream2_get_bytes_left(gb));
+    err = av_dynamic_hdr_plus_from_t35(metadata, gb->buffer,
+                                       bytestream2_get_bytes_left(gb));
     if (err < 0) {
         av_free(metadata);
         return err;
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index eb1225ea1a..50c4ceee40 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -24,6 +24,7 @@ 
 #include "libavutil/avassert.h"
 #include "libavutil/cpu.h"
 #include "libavutil/film_grain_params.h"
+#include "libavutil/hdr_dynamic_metadata.h"
 #include "libavutil/mastering_display_metadata.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/opt.h"
@@ -33,7 +34,6 @@ 
 #include "bytestream.h"
 #include "codec_internal.h"
 #include "decode.h"
-#include "dynamic_hdr10_plus.h"
 #include "internal.h"
 
 #define FF_DAV1D_VERSION_AT_LEAST(x,y) \
@@ -555,8 +555,8 @@  FF_ENABLE_DEPRECATION_WARNINGS
                 goto fail;
             }
 
-            res = ff_parse_itu_t_t35_to_dynamic_hdr10_plus(hdrplus, gb.buffer,
-                                                           bytestream2_get_bytes_left(&gb));
+            res = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer,
+                                               bytestream2_get_bytes_left(&gb));
             if (res < 0)
                 goto fail;
             break;
diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
index 0fa1ee82de..5ed903f475 100644
--- a/libavutil/hdr_dynamic_metadata.c
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -20,6 +20,18 @@ 
 
 #include "hdr_dynamic_metadata.h"
 #include "mem.h"
+#include "libavcodec/get_bits.h"
+#include "libavcodec/put_bits.h"
+
+#define T35_PAYLOAD_MAX_SIZE 907
+
+static const int64_t luminance_den = 1;
+static const int32_t peak_luminance_den = 15;
+static const int64_t rgb_den = 100000;
+static const int32_t fraction_pixel_den = 1000;
+static const int32_t knee_point_den = 4095;
+static const int32_t bezier_anchor_den = 1023;
+static const int32_t saturation_weight_den = 8;
 
 AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size)
 {
@@ -45,3 +57,176 @@  AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame)
 
     return (AVDynamicHDRPlus *)side_data->data;
 }
+
+int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
+                                 size_t size)
+{
+    uint8_t padded_buf[T35_PAYLOAD_MAX_SIZE + AV_INPUT_BUFFER_PADDING_SIZE];
+    GetBitContext gbc, *gb = &gbc;
+    int ret;
+
+    if (!s)
+        return AVERROR(ENOMEM);
+
+    memcpy(padded_buf, data, size);
+
+    ret = init_get_bits8(gb, padded_buf, size);
+    if (ret < 0)
+        return ret;
+
+    if (get_bits_left(gb) < 10)
+        return AVERROR_INVALIDDATA;
+
+    s->application_version = get_bits(gb, 8);
+    s->num_windows = get_bits(gb, 2);
+
+    if (s->num_windows < 1 || s->num_windows > 3) {
+        return AVERROR_INVALIDDATA;
+    }
+
+    if (get_bits_left(gb) < ((19 * 8 + 1) * (s->num_windows - 1)))
+        return AVERROR_INVALIDDATA;
+
+    for (int w = 1; w < s->num_windows; w++) {
+        // The corners are set to absolute coordinates here. They should be
+        // converted to the relative coordinates (in [0, 1]) in the decoder.
+        AVHDRPlusColorTransformParams *params = &s->params[w];
+        params->window_upper_left_corner_x =
+            (AVRational){get_bits(gb, 16), 1};
+        params->window_upper_left_corner_y =
+            (AVRational){get_bits(gb, 16), 1};
+        params->window_lower_right_corner_x =
+            (AVRational){get_bits(gb, 16), 1};
+        params->window_lower_right_corner_y =
+            (AVRational){get_bits(gb, 16), 1};
+
+        params->center_of_ellipse_x = get_bits(gb, 16);
+        params->center_of_ellipse_y = get_bits(gb, 16);
+        params->rotation_angle = get_bits(gb, 8);
+        params->semimajor_axis_internal_ellipse = get_bits(gb, 16);
+        params->semimajor_axis_external_ellipse = get_bits(gb, 16);
+        params->semiminor_axis_external_ellipse = get_bits(gb, 16);
+        params->overlap_process_option = get_bits1(gb);
+    }
+
+    if (get_bits_left(gb) < 28)
+        return AVERROR_INVALIDDATA;
+
+    s->targeted_system_display_maximum_luminance =
+        (AVRational){get_bits_long(gb, 27), luminance_den};
+    s->targeted_system_display_actual_peak_luminance_flag = get_bits1(gb);
+
+    if (s->targeted_system_display_actual_peak_luminance_flag) {
+        int rows, cols;
+        if (get_bits_left(gb) < 10)
+            return AVERROR_INVALIDDATA;
+        rows = get_bits(gb, 5);
+        cols = get_bits(gb, 5);
+        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
+            return AVERROR_INVALIDDATA;
+        }
+        s->num_rows_targeted_system_display_actual_peak_luminance = rows;
+        s->num_cols_targeted_system_display_actual_peak_luminance = cols;
+
+        if (get_bits_left(gb) < (rows * cols * 4))
+            return AVERROR_INVALIDDATA;
+
+        for (int i = 0; i < rows; i++) {
+            for (int j = 0; j < cols; j++) {
+                s->targeted_system_display_actual_peak_luminance[i][j] =
+                    (AVRational){get_bits(gb, 4), peak_luminance_den};
+            }
+        }
+    }
+    for (int w = 0; w < s->num_windows; w++) {
+        AVHDRPlusColorTransformParams *params = &s->params[w];
+        if (get_bits_left(gb) < (3 * 17 + 17 + 4))
+            return AVERROR_INVALIDDATA;
+
+        for (int i = 0; i < 3; i++) {
+            params->maxscl[i] =
+                (AVRational){get_bits(gb, 17), rgb_den};
+        }
+        params->average_maxrgb =
+            (AVRational){get_bits(gb, 17), rgb_den};
+        params->num_distribution_maxrgb_percentiles = get_bits(gb, 4);
+
+        if (get_bits_left(gb) <
+            (params->num_distribution_maxrgb_percentiles * 24))
+            return AVERROR_INVALIDDATA;
+
+        for (int i = 0; i < params->num_distribution_maxrgb_percentiles; i++) {
+            params->distribution_maxrgb[i].percentage = get_bits(gb, 7);
+            params->distribution_maxrgb[i].percentile =
+                (AVRational){get_bits(gb, 17), rgb_den};
+        }
+
+        if (get_bits_left(gb) < 10)
+            return AVERROR_INVALIDDATA;
+
+        params->fraction_bright_pixels = (AVRational){get_bits(gb, 10), fraction_pixel_den};
+    }
+    if (get_bits_left(gb) < 1)
+        return AVERROR_INVALIDDATA;
+    s->mastering_display_actual_peak_luminance_flag = get_bits1(gb);
+    if (s->mastering_display_actual_peak_luminance_flag) {
+        int rows, cols;
+        if (get_bits_left(gb) < 10)
+            return AVERROR_INVALIDDATA;
+        rows = get_bits(gb, 5);
+        cols = get_bits(gb, 5);
+        if (((rows < 2) || (rows > 25)) || ((cols < 2) || (cols > 25))) {
+            return AVERROR_INVALIDDATA;
+        }
+        s->num_rows_mastering_display_actual_peak_luminance = rows;
+        s->num_cols_mastering_display_actual_peak_luminance = cols;
+
+        if (get_bits_left(gb) < (rows * cols * 4))
+            return AVERROR_INVALIDDATA;
+
+        for (int i = 0; i < rows; i++) {
+            for (int j = 0; j < cols; j++) {
+                s->mastering_display_actual_peak_luminance[i][j] =
+                    (AVRational){get_bits(gb, 4), peak_luminance_den};
+            }
+        }
+    }
+
+    for (int w = 0; w < s->num_windows; w++) {
+        AVHDRPlusColorTransformParams *params = &s->params[w];
+        if (get_bits_left(gb) < 1)
+            return AVERROR_INVALIDDATA;
+
+        params->tone_mapping_flag = get_bits1(gb);
+        if (params->tone_mapping_flag) {
+            if (get_bits_left(gb) < 28)
+                return AVERROR_INVALIDDATA;
+
+            params->knee_point_x =
+                (AVRational){get_bits(gb, 12), knee_point_den};
+            params->knee_point_y =
+                (AVRational){get_bits(gb, 12), knee_point_den};
+            params->num_bezier_curve_anchors = get_bits(gb, 4);
+
+            if (get_bits_left(gb) < (params->num_bezier_curve_anchors * 10))
+                return AVERROR_INVALIDDATA;
+
+            for (int i = 0; i < params->num_bezier_curve_anchors; i++) {
+                params->bezier_curve_anchors[i] =
+                    (AVRational){get_bits(gb, 10), bezier_anchor_den};
+            }
+        }
+
+        if (get_bits_left(gb) < 1)
+            return AVERROR_INVALIDDATA;
+        params->color_saturation_mapping_flag = get_bits1(gb);
+        if (params->color_saturation_mapping_flag) {
+            if (get_bits_left(gb) < 6)
+                return AVERROR_INVALIDDATA;
+            params->color_saturation_weight =
+                (AVRational){get_bits(gb, 6), saturation_weight_den};
+        }
+    }
+
+    return 0;
+}
diff --git a/libavutil/hdr_dynamic_metadata.h b/libavutil/hdr_dynamic_metadata.h
index 2d72de56ae..faa4f46b0b 100644
--- a/libavutil/hdr_dynamic_metadata.h
+++ b/libavutil/hdr_dynamic_metadata.h
@@ -340,4 +340,17 @@  AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size);
  */
 AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
 
+/**
+ * Parse the user data registered ITU-T T.35 to AVbuffer (AVDynamicHDRPlus).
+ * The T.35 buffer must begin with the application mode, skipping the
+ * country code, terminal provider codes, and application identifier.
+ * @param s A pointer containing the decoded AVDynamicHDRPlus structure.
+ * @param data The byte array containing the raw ITU-T T.35 data.
+ * @param size Size of the data array in bytes.
+ *
+ * @return 0 if succeed. Otherwise, returns the appropriate AVERROR.
+ */
+int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s, const uint8_t *data,
+                                 size_t size);
+
 #endif /* AVUTIL_HDR_DYNAMIC_METADATA_H */