[FFmpeg-devel] [PATCH] avcodec/cbs_h265: add support for Alpha Channel Info SEI messages
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Sat Jun 22 09:32:00 EEST 2019
Hello,
first a general note: hevc_parse_nal_header in h2645_parse.c
explicitly discards any NAL units with nuh_layer_id != 0. As long as
this is so adding support for this SEI is pointless.
(Your comment on IRC was wrong: The Apple samples contain two SPS and
PPS each.)
James Almer:
> As defined in section F.14.2.8 and F.14.3.8
>
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
> https://trac.ffmpeg.org/attachment/ticket/7965/puppets_with_alpha_hevc.mov
>
> libavcodec/cbs_h2645.c | 1 +
> libavcodec/cbs_h265.h | 12 +++++++++
> libavcodec/cbs_h265_syntax_template.c | 37 +++++++++++++++++++++++++++
> libavcodec/hevc_sei.h | 1 +
> 4 files changed, 51 insertions(+)
>
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index 0456937710..f35a2c01f7 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -530,6 +530,7 @@ static void cbs_h265_free_sei_payload(H265RawSEIPayload *payload)
> case HEVC_SEI_TYPE_MASTERING_DISPLAY_INFO:
> case HEVC_SEI_TYPE_CONTENT_LIGHT_LEVEL_INFO:
> case HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS:
> + case HEVC_SEI_TYPE_ALPHA_CHANNEL_INFO:
> break;
> case HEVC_SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35:
> av_buffer_unref(&payload->payload.user_data_registered.data_ref);
> diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h
> index c9bc90187b..ad746bf35f 100644
> --- a/libavcodec/cbs_h265.h
> +++ b/libavcodec/cbs_h265.h
> @@ -679,6 +679,17 @@ typedef struct H265RawSEIAlternativeTransferCharacteristics {
> uint8_t preferred_transfer_characteristics;
> } H265RawSEIAlternativeTransferCharacteristics;
>
> +typedef struct H265RawSEIAlphaChannelInfo {
> + uint8_t alpha_channel_cancel_flag;
> + uint8_t alpha_channel_use_idc;
> + uint8_t alpha_channel_bit_depth_minus8;
> + uint16_t alpha_transparent_value;
> + uint16_t alpha_opaque_value;
> + uint8_t alpha_channel_incr_flag;
> + uint8_t alpha_channel_clip_flag;
> + uint8_t alpha_channel_clip_type_flag;
> +} H265RawSEIAlphaChannelInfo;
> +
> typedef struct H265RawSEIPayload {
> uint32_t payload_type;
> uint32_t payload_size;
> @@ -697,6 +708,7 @@ typedef struct H265RawSEIPayload {
> H265RawSEIContentLightLevelInfo content_light_level;
> H265RawSEIAlternativeTransferCharacteristics
> alternative_transfer_characteristics;
> + H265RawSEIAlphaChannelInfo alpha_channel_info;
> struct {
> uint8_t *data;
> size_t data_length;
> diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
> index f279d283d9..bcddd6d3b2 100644
> --- a/libavcodec/cbs_h265_syntax_template.c
> +++ b/libavcodec/cbs_h265_syntax_template.c
> @@ -2028,6 +2028,42 @@ static int FUNC(sei_alternative_transfer_characteristics)(CodedBitstreamContext
> return 0;
> }
>
> +static int FUNC(sei_alpha_channel_info)(CodedBitstreamContext *ctx,
> + RWContext *rw,
> + H265RawSEIAlphaChannelInfo *current)
> +{
> + CodedBitstreamH265Context *h265 = ctx->priv_data;
> + const H265RawSPS *sps = h265->active_sps;
> + int err, length;
> +
> + HEADER("Alpha Channel Information");
> +
> + if (!sps) {
> + av_log(ctx->log_ctx, AV_LOG_ERROR,
> + "No active SPS for alpha_channel_info.\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + flag(alpha_channel_cancel_flag);
> + if(!current->alpha_channel_cancel_flag) {
Nit: Missing space after if.
> + ub(3, alpha_channel_use_idc);
> + u(3, alpha_channel_bit_depth_minus8, 0, sps->bit_depth_luma_minus8);
This check is possibly problematic: Think of a file containing more
than one set of parameter sets in its extradata. Given that no coded
picture has been encountered yet, no parameter set is active according
to the standard. But that is not what cbs currently does: Every time a
new PPS is encountered, the SPS referenced therein is considered the
new active SPS. And this can be wrong and in this case it might lead
to valid files being rejected. So I don't think that it is wise to
perform any check on the value of alpha_channel_bit_depth_minus8.
(Btw: Actually, alpha_channel_bit_depth_minus8 has to be equal to
bit_depth_luma_minus8 of the primary coded picture, so if you want to
check, you can also use the lower bound. (I also wonder whether it is
really intended that alpha_channel_bit_depth_minus8 shall be equal to
bit_depth_luma_minus8 of the primary coded picture and not the
auxiliary coded picture that actually contains the alpha channel. Is
it actually legal for bit_depth_luma_minus8 of the auxiliary and the
primary coded picture to differ in this case? I have found nothing
that says that it is illegal.))
> + length = current->alpha_channel_bit_depth_minus8 + 9;
> + ub(length, alpha_transparent_value);
> + ub(length, alpha_opaque_value);
> + flag(alpha_channel_incr_flag);
> + flag(alpha_channel_clip_flag);
> + if(current->alpha_channel_clip_flag)
> + flag(alpha_channel_clip_type_flag);
> + } else {
> + infer(alpha_channel_use_idc, 2);
Nit: The 2 can be aligned with the 0.
> + infer(alpha_channel_incr_flag, 0);
> + infer(alpha_channel_clip_flag, 0);
> + }
> +
> + return 0;
> +}
> +
> static int FUNC(sei_payload)(CodedBitstreamContext *ctx, RWContext *rw,
> H265RawSEIPayload *current, int prefix)
> {
> @@ -2080,6 +2116,7 @@ static int FUNC(sei_payload)(CodedBitstreamContext *ctx, RWContext *rw,
> SEI_TYPE_N(CONTENT_LIGHT_LEVEL_INFO, 1, 0, content_light_level);
> SEI_TYPE_N(ALTERNATIVE_TRANSFER_CHARACTERISTICS,
> 1, 0, alternative_transfer_characteristics);
> + SEI_TYPE_N(ALPHA_CHANNEL_INFO, 1, 0, alpha_channel_info);
>
> #undef SEI_TYPE
> default:
> diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
> index 2fec00ace0..f6516ae982 100644
> --- a/libavcodec/hevc_sei.h
> +++ b/libavcodec/hevc_sei.h
> @@ -56,6 +56,7 @@ typedef enum {
> HEVC_SEI_TYPE_MASTERING_DISPLAY_INFO = 137,
> HEVC_SEI_TYPE_CONTENT_LIGHT_LEVEL_INFO = 144,
> HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS = 147,
> + HEVC_SEI_TYPE_ALPHA_CHANNEL_INFO = 165,
> } HEVC_SEI_Type;
>
> typedef struct HEVCSEIPictureHash {
>
- Andreas
More information about the ffmpeg-devel
mailing list