James Almer: > Will be reused in the following patch. > > Signed-off-by: James Almer <jamr...@gmail.com> > --- > libavcodec/cbs_h2645.c | 9 +++++++++ > libavcodec/cbs_h265_syntax_template.c | 7 +++---- > 2 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c > index d42073cc5a..a60b3217a0 100644 > --- a/libavcodec/cbs_h2645.c > +++ b/libavcodec/cbs_h2645.c > @@ -233,6 +233,15 @@ static int cbs_write_se_golomb(CodedBitstreamContext > *ctx, PutBitContext *pbc, > return 0; > } > > +static int cbs_h265_payload_extension_present(GetBitContext *gbc, uint32_t > payload_size, > + int cur_pos) > +{ > + int bits_left; > + 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)));
After thinking a bit more about HEVC's reserved_payload_extension_data mechanism I came to realize that it has two issues: The first is that it allows to create SEI messages compliant with the current standard, yet incompliant with the older one. Take the buffering period SEI. In the current version of the standard it begins with the syntax elements that said SEI always had followed by this: if( payload_extension_present( ) ) use_alt_cpb_params_flag (a one-bit flag) which is followed by the following generic stuff for all SEI messages: if( more_data_in_payload( ) ) { if( payload_extension_present( ) ) reserved_payload_extension_data payload_bit_equal_to_one /* equal to 1 */ while( !byte_aligned( ) ) payload_bit_equal_to_zero /* equal to 0 */ } The first problem is in the check more_data_in_payload( ). It's placement after payload_extension_present() implies that it is possible to create SEI messages that are valid for the current version of the standard, yet are invalid data for a decoder implementing an older version of the standard. This is possible if the position of the bitstream before use_alt_cpb_params_flag() is 7 mod 8. If the next bit is a bit equal to one and if it is the last bit of the SEI, then use_alt_cpb_params_flag is not present for both a decoder implementing the old as well as a decoder implementing the new version of the standard (this bit will be the payload_bit_equal_to_one for both decoders); yet if this bit is zero, then payload_extension_present() is true and the new decoder will therefore infer that use_alt_cpb_params_flag (with the value zero) is present. It is perfectly legal according to the new standard to end the SEI message at this point (for a new decoder, the more_data_in_payload() check will be false), but the more_data_in_payload() check will be true for an old decoder (because for it there still is one bit left in the SEI message); yet the data left does not contain a single bit equal to one, so that the SEI message is invalid for the old decoder. In this case this problem is avoidable if the new encoder simply does not write the use_alt_cpb_params_flag at all (if it is absent, zero should be inferred anyway); yet this solution depends upon the inferred value to be equal to zero. Similarly it is impossible for an older decoder to know whether the bit it thinks to be the payload_bit_equal_to_one is really the payload_bit_equal_to_one according to a newer standard (it might be that a new syntax element has been added to an SEI message in a newer version of the standard and that the SEI message as seen by the new standard is automatically byte-aligned, so that no payload_bit_equal_to_one is needed according to the new standard). Granted, this is not really important, because the old decoder is supposed to ignore the extension data anyway. (We should of course avoid this problem altogether by always writing the payload_bit_equal_to_one if we have written any field that has been added in a recent version of the standard to an already existing SEI message type.) The second problem is the actual definition of payload_extension_present(): "If the current position in the sei_payload( ) syntax structure is not the position of the last (least significant, right-most) bit that is equal to 1 that is less than 8 * payloadSize bits from the beginning of the syntax structure (i.e., the position of the payload_bit_equal_to_one syntax element), the return value of payload_extension_present( ) is equal to TRUE." So if one is already at the end (i.e. 8 * payloadSize bits from the beginning of the syntax structure away), then payload_extension_present() automatically returns true. So a buffering period SEI message unit without payload_bit_equal_to_one (because what has been written was already byte-aligned) written by an old encoder will be considered invalid according to the new standard if one takes the new standard literally (according to the new standard, use_alt_cpb_params_flag is present because payload_extension_present() is true at the end; yet there is no data left). This also means that the above check in cbs_h265_payload_extension_present() is spec-incompliant. To make it spec-compliant, one would need to return 1 if no bits are left. Of course, we should not make it spec-compliant. I wonder whether the first issue is an intentional micro-optimization designed for systems where only new decoders are used or if it was an oversight. The second issue makes me believe that the first was an oversight. - 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".