[FFmpeg-devel] [PATCH] Update HDR10+ metadata structure.

[Moritz Barsnick]

Hi Mohammad,

On Tue, Feb 04, 2020 at 18:44:00 -0800, Mohammad Izadi wrote:
> --- a/libavutil/hdr_dynamic_metadata.c
> +++ b/libavutil/hdr_dynamic_metadata.c
> @@ -1,4 +1,4 @@
> -/**
> + /**

Please review your git diff and your submitted patches carefully. You
need to avoid this accidental change.

> +    if(!data)

ffmpeg style: "if ("

> +    if(ret < 0)
> +        return AVERROR_INVALIDDATA;

Ditto.

> +    if (get_bits_left(&gb) < 2)
> +        return AVERROR_INVALIDDATA;
> +    s->num_windows = get_bits(&gb, 2);
> +
> +    if (s->num_windows < 1 || s->num_windows > 3) {
> +        return AVERROR_INVALIDDATA;
> +    }

Above, you skip the brackets for the one-liner return. Here, you use
them. That's inconsistent.

> --- a/libavutil/hdr_dynamic_metadata.h
> +++ b/libavutil/hdr_dynamic_metadata.h
> @@ -23,6 +23,8 @@
>
>  #include "frame.h"
>  #include "rational.h"
> +#include "libavcodec/get_bits.h"
> +#include "libavcodec/put_bits.h"

As the header doesn't use functions from these, but the implementation
does, they should be in libavutil/hdr_dynamic_metadata.c instead.

>      /**
>       * The nominal maximum display luminance of the targeted system display,
> -     * in units of 0.0001 candelas per square metre. The value shall be in
> +     * in units of 1 candelas per square metre. The value shall be in
>       * the range of 0 to 10000, inclusive.
>       */

This fix is probably not strictly related to your change?

Cheers,
Moritz