On 4/30/2020 4:15 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 4/30/2020 3:50 PM, Andreas Rheinhardt wrote:
>>> James Almer:
>>>> Fixes ticket #8622
>>>>
>>>> Signed-off-by: James Almer <jamr...@gmail.com>
>>>> ---
>>>> In writing scenarios, it will now ensure bit_equal_to_one is also always
>>>> written when currently defined extension data (like in Buffering Period) is
>>>> present, and not just when unknown extension data is.
>>>>
>>>> libavcodec/cbs_h2645.c | 1 +
>>>> libavcodec/cbs_h265.h | 1 +
>>>> libavcodec/cbs_h265_syntax_template.c | 71 +++++++++++++++++++++++++--
>>>> 3 files changed, 68 insertions(+), 5 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> ---
>>>> a/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..45838467b7 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, end_position;
>>>> + int more_data = !!current->extension_data.bit_length;
>>>
>>> If I am not mistaken, then more_data is a write-only variable when
>>> reading. Better add av_unused or it might lead to compiler warnings.
>>> (You might even move the definition of more_data into the ifdef block
>>> below and only initialize it during writing.)
>>
>> more_data is used as an argument when calling SEI messages using the new
>> SEI_TYPE_E() macro, so that should prevent such warnings. I at least
>> haven't seen any in my tests.
>>
> But none of these functions will actually use it during reading and
> therefore a smart compiler (maybe a future one) might find out that this
> variable is write-only.
Then we'll take care of it when overtly pedantic compilers start
emitting such warnings.
Right now, we have several functions in the tree where function
arguments are unused, so I wouldn't worry.
>
> - Andreas
> _______________________________________________
> 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".
>
_______________________________________________
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".
