[FFmpeg-devel] [PATCH] avcodec/cbs_h265: add support for Alpha Channel Info SEI messages
James Almer
jamrial at gmail.com
Sat Jun 22 16:47:12 EEST 2019
On 6/22/2019 3:32 AM, Andreas Rheinhardt wrote:
> 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.
If you try this patch with the sample i linked, you'll see the SEI is
parsed and shown.
> (Your comment on IRC was wrong: The Apple samples contain two SPS and
> PPS each.)
Ah, that would explain why trace_headers didn't show them.
CBS should probably stop using h2645_parse functions then, and duplicate
its functionality. I'd rather not change h2645_parse for this and risk
unpredictable behavior from the decoder.
>
> 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.
Ok, will remove the check.
>
> (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.
Will change.
>> + 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
More information about the ffmpeg-devel
mailing list