[FFmpeg-devel] [PATCH v4 17/21] vaapi_encode_h264: Support stereo 3D metadata

[Fu, Linjie]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Mark Thompson
> Sent: Monday, February 24, 2020 07:41
> To: ffmpeg-devel at ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v4 17/21] vaapi_encode_h264: Support
> stereo 3D metadata
> 
> Insert frame packing arrangement messages into the stream when input
> frames have associated stereo 3D side-data.
> ---
>  doc/encoders.texi              |  3 +++
>  libavcodec/vaapi_encode_h264.c | 25 ++++++++++++++++++++++++-
>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index e23b6b32fe..62b6902197 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -3065,6 +3065,9 @@ Include picture timing parameters
> (@emph{buffering_period} and
>  @emph{pic_timing} messages).
>  @item recovery_point
>  Include recovery points where appropriate (@emph{recovery_point}
> messages).
> + at item frame_packing
> +Include stereo 3D metadata if the input frames have it
> +(@emph{frame_packing_arrangement} messages).
>  @end table
> 
>  @end table
> diff --git a/libavcodec/vaapi_encode_h264.c
> b/libavcodec/vaapi_encode_h264.c
> index f4965d8b09..58eae613c4 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -25,6 +25,7 @@
>  #include "libavutil/common.h"
>  #include "libavutil/internal.h"
>  #include "libavutil/opt.h"
> +#include "libavutil/stereo3d.h"
> 
>  #include "avcodec.h"
>  #include "cbs.h"
> @@ -39,6 +40,7 @@ enum {
>      SEI_TIMING         = 0x01,
>      SEI_IDENTIFIER     = 0x02,
>      SEI_RECOVERY_POINT = 0x04,
> +    SEI_FRAME_PACKING  = 0x20,
>  };

There is a jumping from 0x04 to 0x20, how about combining it with the enum in
vaapi_encode_h265.c, and moving into vaapi_encode.h, hence SEI_FRAME_PACKING
could also be used for H265 later?

vaapi_encode_h265.c:
enum {
    SEI_MASTERING_DISPLAY       = 0x08,
    SEI_CONTENT_LIGHT_LEVEL     = 0x10,
};

>  // Random (version 4) ISO 11578 UUID.
> @@ -96,6 +98,7 @@ typedef struct VAAPIEncodeH264Context {
>      H264RawSEIBufferingPeriod      sei_buffering_period;
>      H264RawSEIPicTiming            sei_pic_timing;
>      H264RawSEIRecoveryPoint        sei_recovery_point;
> +    H264RawSEIFramePackingArrangement sei_frame_packing;
>      H264RawSEIUserDataUnregistered sei_identifier;
>      char                          *sei_identifier_string;
> 
> @@ -251,6 +254,12 @@ static int
> vaapi_encode_h264_write_extra_header(AVCodecContext *avctx,
>              sei->payload[i].payload.recovery_point = priv->sei_recovery_point;
>              ++i;
>          }
> +        if (priv->sei_needed & SEI_FRAME_PACKING) {
> +            sei->payload[i].payload_type = H264_SEI_TYPE_FRAME_PACKING;
> +            sei->payload[i].payload.frame_packing_arrangement =
> +                priv->sei_frame_packing;
> +            ++i;
> +        }
> 
>          sei->payload_count = i;
>          av_assert0(sei->payload_count > 0);
> @@ -700,6 +709,17 @@ static int
> vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
>          priv->sei_needed |= SEI_RECOVERY_POINT;
>      }
> 
> +    if (priv->sei & SEI_FRAME_PACKING) {
> +        AVFrameSideData *sd = av_frame_get_side_data(pic->input_image,
> +                                                     AV_FRAME_DATA_STEREO3D);
> +        if (sd) {
> +            ff_cbs_h264_fill_sei_frame_packing_arrangement(
> +                &priv->sei_frame_packing, (const AVStereo3D*)sd->data);
> +        }
> +
> +        priv->sei_needed |= SEI_FRAME_PACKING;

If got NULL sd from av_frame_get_side_data(),  would it be better to not adding
SEI_FRAME_PACKING to  priv->sei_needed or taking further actions to write extra header?

- Linjie