On 30/04/2020 22:50, James Almer wrote:
> Fixes ticket #8622
>
> Signed-off-by: James Almer <jamr...@gmail.com>
> ---
> libavcodec/cbs_h2645.c | 1 +
> libavcodec/cbs_h265.h | 1 +
> libavcodec/cbs_h265_syntax_template.c | 85 +++++++++++++++++++++------
> 3 files changed, 70 insertions(+), 17 deletions(-)
>
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index 095e449ddc..b432921ecc 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -553,6 +553,7 @@ static void cbs_h265_free_sei_payload(H265RawSEIPayload
> *payload)
> av_buffer_unref(&payload->payload.other.data_ref);
> break;
> }
> + av_buffer_unref(&payload->extension_data.data_ref);
> }
>
> static void cbs_h265_free_sei(void *opaque, uint8_t *content)
> diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h
> index 2c1e153ad9..73897f77a4 100644
> --- a/libavcodec/cbs_h265.h
> +++ b/libavcodec/cbs_h265.h
> @@ -715,6 +715,7 @@ typedef struct H265RawSEIPayload {
> AVBufferRef *data_ref;
> } other;
> } payload;
> + H265RawExtensionData extension_data;
> } H265RawSEIPayload;
>
> typedef struct H265RawSEI {
> diff --git a/libavcodec/cbs_h265_syntax_template.c
> b/libavcodec/cbs_h265_syntax_template.c
> index ed08b06e9c..d3ac618db6 100644
> --- a/libavcodec/cbs_h265_syntax_template.c
> +++ b/libavcodec/cbs_h265_syntax_template.c
> @@ -1564,7 +1564,8 @@ static int
> FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw,
>
> static int FUNC(sei_buffering_period)(CodedBitstreamContext *ctx, RWContext
> *rw,
> H265RawSEIBufferingPeriod *current,
> - uint32_t *payload_size)
> + uint32_t *payload_size,
> + int *more_data)
> {
> CodedBitstreamH265Context *h265 = ctx->priv_data;
> const H265RawSPS *sps;
> @@ -1658,8 +1659,15 @@ static int
> FUNC(sei_buffering_period)(CodedBitstreamContext *ctx, RWContext *rw,
> else
> infer(use_alt_cpb_params_flag, 0);
> #else
> - if (current->use_alt_cpb_params_flag)
> + // If unknown extension data exists, then use_alt_cpb_params_flag is
> + // coded in the bitstream and must be written even if it's 0.
> + if (current->use_alt_cpb_params_flag || *more_data) {
> flag(use_alt_cpb_params_flag);
> + // Ensure this bit is not the last in the payload by making the
> + // more_data_in_payload() check evaluate to true, so it may not
> + // be mistaken as something else by decoders.
> + *more_data = 1;
> + }
> #endif
>
> return 0;
> @@ -2057,11 +2065,48 @@ static int
> FUNC(sei_alpha_channel_info)(CodedBitstreamContext *ctx,
> return 0;
> }
>
> +static int FUNC(payload_extension)(CodedBitstreamContext *ctx, RWContext *rw,
> + H265RawExtensionData *current, uint32_t
> payload_size,
> + int cur_pos)
> +{
> + int err;
> + size_t byte_length, k;
> +
> +#ifdef READ
> + GetBitContext tmp;
> + int bits_left, payload_zero_bits;
> +
> + if (!cbs_h265_payload_extension_present(rw, payload_size, cur_pos))
> + return 0;
> +
> + bits_left = 8 * payload_size - cur_pos;
> + tmp = *rw;
> + if (bits_left > 8)
> + skip_bits_long(&tmp, bits_left - 8);
> + payload_zero_bits = get_bits(&tmp, FFMIN(bits_left, 8));
> + if (!payload_zero_bits)
> + return AVERROR_INVALIDDATA;
> + payload_zero_bits = ff_ctz(payload_zero_bits);
> + current->bit_length = bits_left - payload_zero_bits - 1;
> + allocate(current->data, (current->bit_length + 7) / 8);
> +#endif
> +
> + byte_length = (current->bit_length + 7) / 8;
> + for (k = 0; k < byte_length; k++) {
> + int length = FFMIN(current->bit_length - k * 8, 8);
> + xu(length, reserved_payload_extension_data, current->data[k],
> + 0, MAX_UINT_BITS(length), 0);
> + }
> +
> + return 0;
> +}
> +
> static int FUNC(sei_payload)(CodedBitstreamContext *ctx, RWContext *rw,
> H265RawSEIPayload *current, int prefix)
> {
> int err, i;
> - int start_position, end_position;
> + int start_position, current_position;
> + int more_data = !!current->extension_data.bit_length;
>
> #ifdef READ
> start_position = get_bits_count(rw);
> @@ -2093,8 +2138,15 @@ static int FUNC(sei_payload)(CodedBitstreamContext
> *ctx, RWContext *rw,
> CHECK(FUNC(sei_ ## name)(ctx, rw, ¤t->payload.name, \
> ¤t->payload_size)); \
> break
> +#define SEI_TYPE_E(type, prefix_valid, suffix_valid, name) \
> + case HEVC_SEI_TYPE_ ## type: \
> + SEI_TYPE_CHECK_VALID(name, prefix_valid, suffix_valid); \
> + CHECK(FUNC(sei_ ## name)(ctx, rw, ¤t->payload.name, \
> + ¤t->payload_size, \
> + &more_data)); \
> + break
>
> - SEI_TYPE_S(BUFFERING_PERIOD, 1, 0, buffering_period);
> + SEI_TYPE_E(BUFFERING_PERIOD, 1, 0, buffering_period);
> SEI_TYPE_N(PICTURE_TIMING, 1, 0, pic_timing);
> SEI_TYPE_N(PAN_SCAN_RECT, 1, 0, pan_scan_rect);
> SEI_TYPE_S(USER_DATA_REGISTERED_ITU_T_T35,
> @@ -2125,24 +2177,23 @@ static int FUNC(sei_payload)(CodedBitstreamContext
> *ctx, RWContext *rw,
> }
> }
>
> - if (byte_alignment(rw)) {
> + // more_data_in_payload()
> +#ifdef READ
> + current_position = get_bits_count(rw) - start_position;
> + if (current_position < 8 * current->payload_size) {
> +#else
> + current_position = put_bits_count(rw) - start_position;
> + if (byte_alignment(rw) || more_data) {
> +#endif
> + CHECK(FUNC(payload_extension)(ctx, rw, ¤t->extension_data,
> + current->payload_size,
> current_position));
> fixed(1, bit_equal_to_one, 1);
> while (byte_alignment(rw))
> fixed(1, bit_equal_to_zero, 0);
> }
>
> -#ifdef READ
> - end_position = get_bits_count(rw);
> - if (end_position < start_position + 8 * current->payload_size) {
> - av_log(ctx->log_ctx, AV_LOG_ERROR, "Incorrect SEI payload length: "
> - "header %"PRIu32" bits, actually %d bits.\n",
> - 8 * current->payload_size,
> - end_position - start_position);
> - return AVERROR_INVALIDDATA;
> - }
> -#else
> - end_position = put_bits_count(rw);
> - current->payload_size = (end_position - start_position) >> 3;
> +#ifdef WRITE
> + current->payload_size = (put_bits_count(rw) - start_position) >> 3;
> #endif
>
> return 0;
>
Could we avoid some of the extra code here by making a new GetBitContext in
sei_payload containing only the payload bits and passing that to the per-type
SEI reading functions? The functions themselves would know whether any
additional bits are present purely from get_bits_left() - buffering_period
could then be a normal SEI_TYPE_N and the code in it would be simpler.
- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
