On Tue, 8 Jan 2019 at 00:26, Mohammad Izadi <moh.iz...@gmail.com> wrote:
> ---
> libavcodec/hevc_sei.c | 236 ++++++++++++++++++++++++++++++++++++++++--
> libavcodec/hevc_sei.h | 6 ++
> libavcodec/hevcdec.c | 19 ++++
> 3 files changed, 255 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
> index c59bd4321e..7e59bf0813 100644
> --- a/libavcodec/hevc_sei.c
> +++ b/libavcodec/hevc_sei.c
> @@ -25,6 +25,7 @@
> #include "golomb.h"
> #include "hevc_ps.h"
> #include "hevc_sei.h"
> +#include "libavutil/hdr_dynamic_metadata.h"
>
> static int decode_nal_sei_decoded_picture_hash(HEVCSEIPictureHash *s,
> GetBitContext *gb)
> {
> @@ -206,10 +207,205 @@ static int
> decode_registered_user_data_closed_caption(HEVCSEIA53Caption *s, GetB
> return 0;
> }
>
> -static int decode_nal_sei_user_data_registered_itu_t_t35(HEVCSEI *s,
> GetBitContext *gb,
> +static int decode_registered_user_data_dynamic_hdr_plus(AVDynamicHDRPlus
> *s, GetBitContext *gb,
> + void *logctx, int
> size)
> +{
> + const int luminance_den = 10000;
> + const int peak_luminance_den = 15;
> + const int rgb_den = 100000;
> + const int fraction_pixel_den = 1000;
> + const int knee_point_den = 4095;
> + const int bezier_anchor_den = 1023;
> + const int saturation_weight_den = 8;
> +
> + int bits_left = size * 8;
> + int w, i, j;
> +
> + if (bits_left < 2)
> + return AVERROR_INVALIDDATA;
> +
> + s->num_windows = get_bits(gb, 2);
> + bits_left -= 2;
>
Remove bits_left. You can check how many bits you've read via
get_bits_count (just keep the start value as an offset to subtract).
+ if (s->num_windows < 1 || s->num_windows > 3) {
> + av_log(logctx, AV_LOG_ERROR, "num_windows=%d, must be in [1,
> 3]\n",
> + s->num_windows);
> + return AVERROR_INVALIDDATA;
> + }
> +
> + if (bits_left < ((19 * 8 + 1) * (s->num_windows - 1)))
> + return AVERROR(EINVAL);
> + for (w = 1; w < s->num_windows; w++) {
> + s->params[w].window_upper_left_corner_x.num = get_bits(gb, 16);
> + s->params[w].window_upper_left_corner_y.num = get_bits(gb, 16);
> + s->params[w].window_lower_right_corner_x.num = get_bits(gb, 16);
> + s->params[w].window_lower_right_corner_y.num = get_bits(gb, 16);
> + // The corners are set to absolute coordinates here. They should
> be
> + // converted to the relative coordinates (in [0, 1]) in the
> decoder.
> + s->params[w].window_upper_left_corner_x.den = 1;
> + s->params[w].window_upper_left_corner_y.den = 1;
> + s->params[w].window_lower_right_corner_x.den = 1;
> + s->params[w].window_lower_right_corner_y.den = 1;
> +
> + s->params[w].center_of_ellipse_x = get_bits(gb, 16);
> + s->params[w].center_of_ellipse_y = get_bits(gb, 16);
> + s->params[w].rotation_angle = get_bits(gb, 8);
> + s->params[w].semimajor_axis_internal_ellipse = get_bits(gb, 16);
> + s->params[w].semimajor_axis_external_ellipse = get_bits(gb, 16);
> + s->params[w].semiminor_axis_external_ellipse = get_bits(gb, 16);
> + s->params[w].overlap_process_option = get_bits(gb, 1);
> + bits_left -= 19 * 8 + 1;
> + }
> +
> + if (bits_left < 28)
> + return AVERROR(EINVAL);
> + s->targeted_system_display_maximum_luminance.num = get_bits(gb, 27);
> + s->targeted_system_display_maximum_luminance.den = luminance_den;
> + s->targeted_system_display_actual_peak_luminance_flag = get_bits(gb,
> 1);
> + bits_left -= 28;
> +
> + if (s->targeted_system_display_actual_peak_luminance_flag) {
> + int rows, cols;
> + if (bits_left < 10)
> + return AVERROR(EINVAL);
> + rows = get_bits(gb, 5);
> + cols = get_bits(gb, 5);
> + if (((rows < 2) && (rows > 25)) || ((cols < 2) && (cols > 25))) {
> + av_log(logctx, AV_LOG_ERROR, "num_rows=%d, num_cols=%d, they
> must "
> + "be in [2, 25] for "
> + "targeted_system_display_actual_peak_luminance\n",
> + rows, cols);
>
Align this better, we don't have strict 80 col line rule.
+ return AVERROR_INVALIDDATA;
> + }
> + s->num_rows_targeted_system_display_actual_peak_luminance = rows;
> + s->num_cols_targeted_system_display_actual_peak_luminance = cols;
> + bits_left -= 10;
> +
> + if (bits_left < (rows * cols * 4))
> + return AVERROR(EINVAL);
> +
> + for (i = 0; i < rows; i++) {
> + for (j = 0; j < cols; j++) {
> +
> s->targeted_system_display_actual_peak_luminance[i][j].num =
> + get_bits(gb, 4);
> +
> s->targeted_system_display_actual_peak_luminance[i][j].den =
> + peak_luminance_den;
>
Same, just put the assignment values on the same line.
+ }
> + }
> + bits_left -= (rows * cols * 4);
> + }
> + for (w = 0; w < s->num_windows; w++) {
> + if (bits_left < (3 * 17 + 17 + 4))
> + return AVERROR(EINVAL);
> + for (i = 0; i < 3; i++) {
> + s->params[w].maxscl[i].num = get_bits(gb, 17);
> + s->params[w].maxscl[i].den = rgb_den;
> + }
> + s->params[w].average_maxrgb.num = get_bits(gb, 17);
> + s->params[w].average_maxrgb.den = rgb_den;
> + s->params[w].num_distribution_maxrgb_percentiles = get_bits(gb,
> 4);
> + bits_left -= (3 * 17 + 17 + 4);
> +
> + if (bits_left <
> + (s->params[w].num_distribution_maxrgb_percentiles * 24))
> + return AVERROR(EINVAL);
> + for (i = 0; i < s->params[w].num_distribution_maxrgb_percentiles;
> i++) {
> + s->params[w].distribution_maxrgb[i].percentage = get_bits(gb,
> 7);
> + s->params[w].distribution_maxrgb[i].percentile.num =
> + get_bits(gb, 17);
>
Same.
> + s->params[w].distribution_maxrgb[i].percentile.den = rgb_den;
> + }
> + bits_left -= (s->params[w].num_distribution_maxrgb_percentiles *
> 24);
> +
> + if (bits_left < 10)
> + return AVERROR(EINVAL);
> + s->params[w].fraction_bright_pixels.num = get_bits(gb, 10);
> + s->params[w].fraction_bright_pixels.den = fraction_pixel_den;
> + bits_left -= 10;
> + }
> + if (bits_left < 1)
> + return AVERROR(EINVAL);
> + s->mastering_display_actual_peak_luminance_flag = get_bits(gb, 1);
> + bits_left--;
> + if (s->mastering_display_actual_peak_luminance_flag) {
> + int rows, cols;
> + if (bits_left < 10)
> + return AVERROR(EINVAL);
> + rows = get_bits(gb, 5);
> + cols = get_bits(gb, 5);
> + if (((rows < 2) && (rows > 25)) || ((cols < 2) && (cols > 25))) {
> + av_log(logctx, AV_LOG_ERROR, "num_rows=%d, num_cols=%d, they
> must "
> + "be in [2, 25] for "
> + "mastering_display_actual_peak_luminance\n",
> + rows, cols);
>
Same.
> + return AVERROR_INVALIDDATA;
> + }
> + s->num_rows_mastering_display_actual_peak_luminance = rows;
> + s->num_cols_mastering_display_actual_peak_luminance = cols;
> + bits_left -= 10;
> +
> + if (bits_left < (rows * cols * 4))
> + return AVERROR(EINVAL);
> +
> + for (i = 0; i < rows; i++) {
> + for (j = 0; j < cols; j++) {
> + s->mastering_display_actual_peak_luminance[i][j].num =
> + get_bits(gb, 4);
> + s->mastering_display_actual_peak_luminance[i][j].den =
> + peak_luminance_den;
>
Same.
> + }
> + }
> + bits_left -= (rows * cols * 4);
> + }
> +
> + for (w = 0; w < s->num_windows; w++) {
> + if (bits_left < 1)
> + return AVERROR(EINVAL);
> + s->params[w].tone_mapping_flag = get_bits(gb, 1);
> + bits_left--;
> + if (s->params[w].tone_mapping_flag) {
> + if (bits_left < 28)
> + return AVERROR(EINVAL);
> + s->params[w].knee_point_x.num = get_bits(gb, 12);
> + s->params[w].knee_point_x.den = knee_point_den;
> + s->params[w].knee_point_y.num = get_bits(gb, 12);
> + s->params[w].knee_point_y.den = knee_point_den;
> + s->params[w].num_bezier_curve_anchors = get_bits(gb, 4);
> + bits_left -= 28;
> +
> + if (bits_left < (s->params[w].num_bezier_curve_anchors * 10))
> + return AVERROR(EINVAL);
> + for (i = 0; i < s->params[w].num_bezier_curve_anchors; i++) {
> + s->params[w].bezier_curve_anchors[i].num = get_bits(gb,
> 10);
> + s->params[w].bezier_curve_anchors[i].den =
> bezier_anchor_den;
> + }
> + bits_left -= (s->params[w].num_bezier_curve_anchors * 10);
> + }
> +
> + if (bits_left < 1)
> + return AVERROR(EINVAL);
> + s->params[w].color_saturation_mapping_flag = get_bits(gb, 1);
> + bits_left--;
> + if (s->params[w].color_saturation_mapping_flag) {
> + if (bits_left < 6)
> + return AVERROR(EINVAL);
> + s->params[w].color_saturation_weight.num = get_bits(gb, 6);
> + s->params[w].color_saturation_weight.den =
> saturation_weight_den;
> + bits_left -= 6;
> + }
> + }
> +
> + skip_bits(gb, bits_left);
> +
> + return 0;
> +}
> +
> +static int decode_nal_sei_user_data_registered_itu_t_t35(HEVCSEI *s,
> + GetBitContext
> *gb,
> + void *logctx,
> int size)
> {
> - uint32_t country_code;
> + uint8_t country_code;
> + uint16_t provider_code;
> uint32_t user_identifier;
>
> if (size < 7)
> @@ -222,11 +418,38 @@ static int
> decode_nal_sei_user_data_registered_itu_t_t35(HEVCSEI *s, GetBitConte
> size--;
> }
>
> - skip_bits(gb, 8);
> - skip_bits(gb, 8);
> -
> + provider_code = get_bits(gb, 16);
> user_identifier = get_bits_long(gb, 32);
>
> + // Check for dynamic metadata - HDR10+(SMPTE 2094-40).
> + if ((provider_code == 0x003C) &&
> + ((user_identifier & 0xFFFFFF00) == 0x00010400)) {
> + int err;
> + size_t size;
> + AVDynamicHDRPlus* hdr_plus = av_dynamic_hdr_plus_alloc(&size);
>
Wrong coding style. We never put the * after the type but always before the
variable.
+ if (!hdr_plus) {
> + return AVERROR(ENOMEM);
> + }
> + s->dynamic_hdr_plus.info =
> + av_buffer_create((uint8_t*)hdr_plus, size,
>
Same if there's no variable, put a space before the *.
> + av_buffer_default_free, NULL, 0);
> + if (!s->dynamic_hdr_plus.info) {
> + av_freep(&hdr_plus);
> + return AVERROR(ENOMEM);
> + }
> +
> + hdr_plus->itu_t_t35_country_code =
> + country_code;
>
Put these on the same line.
+ hdr_plus->application_version =
> + (uint8_t)((user_identifier & 0x000000FF));
> +
> + err = decode_registered_user_data_dynamic_hdr_plus(hdr_plus, gb,
> logctx, size);
> + if (!err){
>
+ av_buffer_unref(&s->dynamic_hdr_plus.info);
> + }
>
No need for brackets for 1-line statements.
+ return err;
> + }
> +
> switch (user_identifier) {
> case MKBETAG('G', 'A', '9', '4'):
> return
> decode_registered_user_data_closed_caption(&s->a53_caption, gb, size);
> @@ -292,7 +515,7 @@ static int decode_nal_sei_prefix(GetBitContext *gb,
> void *logctx, HEVCSEI *s,
> case HEVC_SEI_TYPE_ACTIVE_PARAMETER_SETS:
> return decode_nal_sei_active_parameter_sets(s, gb, logctx);
> case HEVC_SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35:
> - return decode_nal_sei_user_data_registered_itu_t_t35(s, gb, size);
> + return decode_nal_sei_user_data_registered_itu_t_t35(s, gb,
> logctx, size);
> case HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS:
> return
> decode_nal_sei_alternative_transfer(&s->alternative_transfer, gb);
> default:
> @@ -365,4 +588,5 @@ void ff_hevc_reset_sei(HEVCSEI *s)
> {
> s->a53_caption.a53_caption_size = 0;
> av_freep(&s->a53_caption.a53_caption);
> + av_buffer_unref(&s->dynamic_hdr_plus.info);
>
Does ff_hevc_reset_sei get called during uninit?
If it doesn't, you'll need to unref it again in the uninit function.
}
> diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
> index 2fec00ace0..3dffeebb9b 100644
> --- a/libavcodec/hevc_sei.h
> +++ b/libavcodec/hevc_sei.h
> @@ -23,6 +23,7 @@
>
> #include <stdint.h>
>
> +#include "libavutil/buffer.h"
> #include "get_bits.h"
>
> /**
> @@ -94,6 +95,10 @@ typedef struct HEVCSEIMasteringDisplay {
> uint32_t min_luminance;
> } HEVCSEIMasteringDisplay;
>
> +typedef struct HEVCSEIDynamicHDRPlus{
> + AVBufferRef* info;
>
Once again, * is in the wrong place.
+} HEVCSEIDynamicHDRPlus;
> +
> typedef struct HEVCSEIContentLight {
> int present;
> uint16_t max_content_light_level;
> @@ -109,6 +114,7 @@ typedef struct HEVCSEI {
> HEVCSEIPictureHash picture_hash;
> HEVCSEIFramePacking frame_packing;
> HEVCSEIDisplayOrientation display_orientation;
> + HEVCSEIDynamicHDRPlus dynamic_hdr_plus;
> HEVCSEIPictureTiming picture_timing;
> HEVCSEIA53Caption a53_caption;
> HEVCSEIMasteringDisplay mastering_display;
> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index 10bf2563c0..2ca76b600d 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -28,6 +28,7 @@
> #include "libavutil/display.h"
> #include "libavutil/internal.h"
> #include "libavutil/mastering_display_metadata.h"
> +#include "libavutil/hdr_dynamic_metadata.h"
> #include "libavutil/md5.h"
> #include "libavutil/opt.h"
> #include "libavutil/pixdesc.h"
> @@ -2769,6 +2770,24 @@ static int set_side_data(HEVCContext *s)
> s->avctx->color_trc = out->color_trc =
> s->sei.alternative_transfer.preferred_transfer_characteristics;
> }
>
> + if (s->sei.dynamic_hdr_plus.info){
> + int w;
> + AVDynamicHDRPlus* metadata = (AVDynamicHDRPlus*)s->
> sei.dynamic_hdr_plus.info;
>
Same.
Also this is wrong. You're casting an AVBufferRef to a AVDynamicHDRPlus.
You need to set this to the AVBufferRef's data field.
+ if (!metadata) return AVERROR(ENOMEM);
>
Put the return on a newline.
+ // Convert coordinates to relative coordinate in [0, 1].
> + w = 0;
>
Why do you have a random assignment here? Just remove it, along with the
variable at the top. We can do for (int loops now.
+ metadata->params[0].window_upper_left_corner_x.num = 0;
> + metadata->params[0].window_upper_left_corner_y.num = 0;
> + metadata->params[0].window_lower_right_corner_x.num =
> out->width-1;
> + metadata->params[0].window_lower_right_corner_y.num =
> out->height-1;
> + for (w = 0; w < metadata->num_windows; w++) {
>
for (int w = 0,
> + metadata->params[w].window_upper_left_corner_x.den =
> out->width-1;
> + metadata->params[w].window_upper_left_corner_y.den =
> out->height-1;
> + metadata->params[w].window_lower_right_corner_x.den =
> out->width-1;
> + metadata->params[w].window_lower_right_corner_y.den =
> out->height-1;
>
Can't these be set during NAL unit parsing? Or can the frame size change
with the hdr data remaining valid for future frames.
+ }
> + }
> +
> return 0;
> }
>
> --
> 2.20.1.97.g81188d93c3-goog
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
I can't see where you attach the AVBufferRef to the frame as side data.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
