On 4/22/2020 9:17 PM, Andreas Rheinhardt wrote:
> James Almer:
>> Fixes ticket #8622
>>
>> Signed-off-by: James Almer <jamr...@gmail.com>
>> ---
>> It fixes it assuming the Picture Timing in that sample is not being misparsed
>> by cbs_h265.
>> We're compliant with the latest revision of the spec, so any
>> reserved_payload_extension_data found in a sample created today is almost
>> guaranteed to be bogus. But the spec states that they should be skipped if
>> found, for forward compatibility reasons.
>> Would be worth checking if the nvenc encoder is at fault, writing faulty SEI
>> messages.
>>
>> This patch could for that matter make many potential cases of undiscovered
>> cbs_h265 SEI misparsing be handled as if the remaining bits were these
>> reserved
>> bits.
>
> That's unfortunately true. I don't see a way to avoid that.
>
>>
>> libavcodec/cbs_h265.h | 1 +
>> libavcodec/cbs_h265_syntax_template.c | 44 +++++++++++++++++++++++++--
>> 2 files changed, 43 insertions(+), 2 deletions(-)
>>
>> 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 f978e16549..ef3254d27f 100644
>> --- a/libavcodec/cbs_h265_syntax_template.c
>> +++ b/libavcodec/cbs_h265_syntax_template.c
>> @@ -2054,11 +2054,43 @@ 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, i;
>> +
>> +#ifdef READ
>> + if (cbs_h265_payload_extension_present(rw, payload_size, cur_pos)) {
>> + GetBitContext tmp = *rw;
>> + int payload_zero_bits, bits_left = 8 * payload_size - cur_pos;
>> + if (bits_left > 8)
>> + skip_bits_long(&tmp, bits_left - 8);
>> + payload_zero_bits = ff_ctz(get_bits(&tmp, FFMIN(bits_left, 8)));
>> + if (payload_zero_bits >= bits_left - 1)
>> + return AVERROR_INVALIDDATA;
>> + current->bit_length = bits_left - payload_zero_bits - 1;
>> + allocate(current->data, (current->bit_length + 7) / 8);
>> + for (i = 0; i < current->bit_length; i++) {
>> + uint8_t bit;
>> + xu(1, reserved_payload_extension_data, bit, 0, 1, 0);
>> + current->data[i / 8] |= bit << (7 - i % 8);
>> + }
>> + }
>> +#else
>> + for (i = 0; i < current->bit_length; i++)
>> + xu(1, reserved_payload_extension_data,
>> + current->data[i / 8] >> (7 - i % 8) & 1, 0, 1, 0);
>
> Do we really need to write one bit at a time? Why not simply one byte at
> a time and then output the rest in one go? The specs treat it as a
> bitfield and not as individual bits, so I think we are free to group it
> as we please.
Mmh, ok.
>
> The same goes for the other extension_data() function, of course. And
> for reading.
Doing it for the other function will affect the existing trace output,
so I'm inclined to leave it as is.
>
> And shouldn't i be better size_t?
The correct type would be ptrdiff_t, but we never bothered with it and
always used int.
>
>> +#endif
>> +
>> + 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;
>>
>> #ifdef READ
>> start_position = get_bits_count(rw);
>> @@ -2122,7 +2154,15 @@ static int FUNC(sei_payload)(CodedBitstreamContext
>> *ctx, RWContext *rw,
>> }
>> }
>>
>> - if (byte_alignment(rw)) {
>> +#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) || current->extension_data.bit_length) {
>> +#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);
>>
>
> _______________________________________________
> 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".
