[FFmpeg-devel] [PATCH 4/4] avcodec/h2645_sei: add support for Ambient Viewing Environment SEI

[James Almer]

On 1/10/2023 6:19 PM, Jan Ekström wrote:
> Defined by H.274, this SEI message is utilized by iPhones to save
> the nominal ambient viewing environment for the display of recorded
> HDR content. The contents of the message are exposed to API users
> as AVFrame side data containing AVAmbientViewingEnvironment.
> 
> As the DV RPU test sample is from an iPhone and includes Ambient
> Viewing Environment SEI messages, its test result gets updated.
> ---
>   libavcodec/h2645_sei.c     | 47 ++++++++++++++++++++++++++++++++++++++
>   libavcodec/h2645_sei.h     |  8 +++++++
>   tests/ref/fate/hevc-dv-rpu | 12 ++++++++++
>   3 files changed, 67 insertions(+)
> 
> diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
> index 3ff89e4fdd..5083079cb1 100644
> --- a/libavcodec/h2645_sei.c
> +++ b/libavcodec/h2645_sei.c
> @@ -25,6 +25,7 @@
>   
>   #include "config_components.h"
>   
> +#include "libavutil/ambient_viewing_environment.h"
>   #include "libavutil/display.h"
>   #include "libavutil/film_grain_params.h"
>   #include "libavutil/pixdesc.h"
> @@ -320,6 +321,31 @@ static int decode_alternative_transfer(H2645SEIAlternativeTransfer *s,
>       return 0;
>   }
>   
> +static int decode_ambient_viewing_environment(H2645SEIAmbientViewingEnvironment *s,
> +                                              GetByteContext *gb)
> +{
> +    static const uint16_t max_ambient_light_value = 50000;
> +
> +    if (bytestream2_get_bytes_left(gb) < 8)
> +        return AVERROR_INVALIDDATA;
> +
> +    s->ambient_illuminance = bytestream2_get_be32u(gb);
> +    if (!s->ambient_illuminance)
> +        return AVERROR_INVALIDDATA;
> +
> +    s->ambient_light_x = bytestream2_get_be16u(gb);
> +    if (s->ambient_light_x > max_ambient_light_value)
> +        return AVERROR_INVALIDDATA;
> +
> +    s->ambient_light_y = bytestream2_get_be16u(gb);
> +    if (s->ambient_light_y > max_ambient_light_value)
> +        return AVERROR_INVALIDDATA;
> +
> +    s->present = 1;
> +
> +    return 0;
> +}
> +
>   static int decode_film_grain_characteristics(H2645SEIFilmGrainCharacteristics *h,
>                                                enum AVCodecID codec_id, GetBitContext *gb)
>   {
> @@ -383,6 +409,9 @@ int ff_h2645_sei_message_decode(H2645SEI *h, enum SEIType type,
>           return decode_frame_packing_arrangement(&h->frame_packing, gb, codec_id);
>       case SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS:
>           return decode_alternative_transfer(&h->alternative_transfer, gbyte);
> +    case SEI_TYPE_AMBIENT_VIEWING_ENVIRONMENT:
> +        return decode_ambient_viewing_environment(&h->ambient_viewing_environment,
> +                                                  gbyte);
>       default:
>           return FF_H2645_SEI_MESSAGE_UNHANDLED;
>       }
> @@ -609,6 +638,20 @@ int ff_h2645_sei_to_frame(AVFrame *frame, H2645SEI *sei,
>               avctx->properties |= FF_CODEC_PROPERTY_FILM_GRAIN;
>       }
>   
> +    if (sei->ambient_viewing_environment.present) {
> +        H2645SEIAmbientViewingEnvironment *env =
> +            &sei->ambient_viewing_environment;
> +
> +        AVAmbientViewingEnvironment *dst_env =
> +            av_ambient_viewing_environment_create_side_data(frame);
> +        if (!dst_env)
> +            return AVERROR(ENOMEM);
> +
> +        dst_env->ambient_illuminance = av_make_q(env->ambient_illuminance, 10000);
> +        dst_env->ambient_light_x     = av_make_q(env->ambient_light_x,     50000);
> +        dst_env->ambient_light_y     = av_make_q(env->ambient_light_y,     50000);
> +    }
> +
>       return 0;
>   }
>   
> @@ -622,4 +665,8 @@ void ff_h2645_sei_reset(H2645SEI *s)
>       av_freep(&s->unregistered.buf_ref);
>       av_buffer_unref(&s->dynamic_hdr_plus.info);
>       av_buffer_unref(&s->dynamic_hdr_vivid.info);
> +
> +    s->ambient_viewing_environment = (H2645SEIAmbientViewingEnvironment){
> +        .present = 0

This is unnecessarily zeroing the whole struct by extension. Just set 
the one field alone.

> +    };
>   }
> diff --git a/libavcodec/h2645_sei.h b/libavcodec/h2645_sei.h
> index f3ee9af524..e07ae10376 100644
> --- a/libavcodec/h2645_sei.h
> +++ b/libavcodec/h2645_sei.h
> @@ -76,6 +76,13 @@ typedef struct H2645SEIAlternativeTransfer {
>       int preferred_transfer_characteristics;
>   } H2645SEIAlternativeTransfer;
>   
> +typedef struct H2645SEIAmbientViewingEnvironment {
> +    int present;
> +    uint32_t ambient_illuminance;
> +    uint16_t ambient_light_x;
> +    uint16_t ambient_light_y;
> +} H2645SEIAmbientViewingEnvironment;
> +
>   typedef struct H2645SEIFilmGrainCharacteristics {
>       int present;
>       int model_id;
> @@ -108,6 +115,7 @@ typedef struct H2645SEI {
>       H2645SEIDisplayOrientation display_orientation;
>       H2645SEIAlternativeTransfer alternative_transfer;
>       H2645SEIFilmGrainCharacteristics film_grain_characteristics;
> +    H2645SEIAmbientViewingEnvironment ambient_viewing_environment;
>   } H2645SEI;
>   
>   enum {
> diff --git a/tests/ref/fate/hevc-dv-rpu b/tests/ref/fate/hevc-dv-rpu
> index 1980ab13ea..aaf0223eab 100644
> --- a/tests/ref/fate/hevc-dv-rpu
> +++ b/tests/ref/fate/hevc-dv-rpu
> @@ -3,6 +3,12 @@
>   side_data_type=H.26[45] User Data Unregistered SEI message
>   [/SIDE_DATA]
>   [SIDE_DATA]
> +side_data_type=Ambient viewing environment
> +ambient_illuminance=3140000/10000
> +ambient_light_x=15635/50000
> +ambient_light_y=16450/50000
> +[/SIDE_DATA]
> +[SIDE_DATA]
>   side_data_type=Dolby Vision RPU Data
>   [/SIDE_DATA]
>   [SIDE_DATA]
> @@ -120,6 +126,12 @@ source_diagonal=42
>   [/FRAME]
>   [FRAME]
>   [SIDE_DATA]
> +side_data_type=Ambient viewing environment
> +ambient_illuminance=3140000/10000
> +ambient_light_x=15635/50000
> +ambient_light_y=16450/50000
> +[/SIDE_DATA]
> +[SIDE_DATA]
>   side_data_type=Dolby Vision RPU Data
>   [/SIDE_DATA]
>   [SIDE_DATA]

LGTM.