[FFmpeg-devel] [PATCH 2/3 v3] avcodec/cbs_h265: move the payload_extension_present check into its own function

[James Almer]

On 4/30/2020 3:08 PM, Andreas Rheinhardt wrote:
> James Almer:
>> Will be reused in the following patch.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> Moved the comment for the function to its new place, but otherwise, no
>> difference compared to v2.
>>
>>  libavcodec/cbs_h2645.c                | 10 ++++++++++
>>  libavcodec/cbs_h265_syntax_template.c |  9 +++------
>>  2 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
>> index d42073cc5a..095e449ddc 100644
>> --- a/libavcodec/cbs_h2645.c
>> +++ b/libavcodec/cbs_h2645.c
>> @@ -233,6 +233,16 @@ static int cbs_write_se_golomb(CodedBitstreamContext *ctx, PutBitContext *pbc,
>>      return 0;
>>  }
>>  
>> +// payload_extension_present() - true if we are before the last 1-bit
>> +// in the payload structure, which must be in the last byte.
>> +static int cbs_h265_payload_extension_present(GetBitContext *gbc, uint32_t payload_size,
>> +                                              int cur_pos)
>> +{
>> +    int bits_left = payload_size * 8 - cur_pos;
>> +    return (bits_left > 0 &&
>> +            (bits_left > 7 || show_bits(gbc, bits_left) & MAX_UINT_BITS(bits_left - 1)));
> Consider a buffering period SEI message whose bit position is 7 mod 8
> before the payload extension present check and where there is only one
> bit, namely a zero bit left in the SEI message. According to the
> definition of payload_extension_present() in the standard, it will
> Consider a buffering period SEI message whose bit position is 7 mod 8
> before the payload extension present check and where there is only one
> bit, namely a zero bit left in the SEI message. According to the
> definition of payload_extension_present() in the standard, it will
> return true and the last bit is use_alt_cpb_params_flag (with a value
> zero); yet nevertheless the more_data_in_payload() check after the
> buffering_period() returns false. This is a valid SEI message according
> to the current version of the standard (in fact, for all versions except
> for the very first version).

I consider a Buffering Period SEI message as you described it as being
invalid precisely because it wont work on all revisions of the spec. The
hypotetical encoder simply did not write the SEI message with backwards
compatibility in mind, which would have been trivial to avoid.

> 
> Yet the check above will return false in this scenario and the SEI
> message unit will be considered invalid. I don't object to this, but you
> should mention this discrepancy in a comment.
> 
> - Andreas
> 
>> +}
>> +
>>  #define HEADER(name) do { \
>>          ff_cbs_trace_header(ctx, name); \
>>      } while (0)
>> diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
>> index a51b12cfc6..ed08b06e9c 100644
>> --- a/libavcodec/cbs_h265_syntax_template.c
>> +++ b/libavcodec/cbs_h265_syntax_template.c
>> @@ -1572,7 +1572,7 @@ static int FUNC(sei_buffering_period)(CodedBitstreamContext *ctx, RWContext *rw,
>>      int err, i, length;
>>  
>>  #ifdef READ
>> -    int start_pos, end_pos, bits_left;
>> +    int start_pos, end_pos;
>>      start_pos = get_bits_count(rw);
>>  #endif
>>  
>> @@ -1651,12 +1651,9 @@ static int FUNC(sei_buffering_period)(CodedBitstreamContext *ctx, RWContext *rw,
>>      }
>>  
>>  #ifdef READ
>> -    // payload_extension_present() - true if we are before the last 1-bit
>> -    // in the payload structure, which must be in the last byte.
>>      end_pos = get_bits_count(rw);
>> -    bits_left = *payload_size * 8 - (end_pos - start_pos);
>> -    if (bits_left > 0 &&
>> -        (bits_left > 7 || ff_ctz(show_bits(rw, bits_left)) < bits_left - 1))
>> +    if (cbs_h265_payload_extension_present(rw, *payload_size,
>> +                                           end_pos - start_pos))
>>          flag(use_alt_cpb_params_flag);
>>      else
>>          infer(use_alt_cpb_params_flag, 0);
>>
> 
> _______________________________________________
> 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".
>