On 7/9/2020 7:35 AM, Andreas Rheinhardt wrote:
> The current code for parsing unsigned exponential-golomb codes is not
> suitable for long codes: If there are enough leading zeroes, it
> left-shifts 1 by 32 places and uses get_bitsz to read 32 bits, although
> it is only suitable to read 0-25 bits. There is an av_assert2 to
> ensure this when using the uncached bitstream reader; with valid input
> one can still run into the assert and left-shift 1 by 31 which is
> undefined.
>
> This commit changes this. All valid data is parsed correctly; invalid
> data does no longer lead to undefined behaviour or to asserts. Parsing
> all valid data correctly also necessitated changing the return value to
> unsigned; also an intermediate variable used for parsing signed
> exponential-golomb codes needed to be made unsigned, too, in order to
> parse long signed codes correctly.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
> ---
> Moving the function to the beginning is done in preparation for another
> commit that I'll send soon.
>
> libavformat/avc.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/libavformat/avc.c b/libavformat/avc.c
> index b5e2921388..55494eb08a 100644
> --- a/libavformat/avc.c
> +++ b/libavformat/avc.c
> @@ -27,6 +27,21 @@
> #include "avc.h"
> #include "avio_internal.h"
>
> +static inline unsigned get_ue_golomb(GetBitContext *gb)
> +{
> + int i;
> + for (i = 1; i <= 32; i++) {
> + if (get_bits_left(gb) < i)
> + return 0;
> + if (show_bits1(gb))
> + break;
> + skip_bits1(gb);
> + }
> + if (i > 32)
> + return 0;
> + return get_bits_long(gb, i) - 1;
> +}
> +
> static const uint8_t *avc_find_startcode_internal(const uint8_t *p, const
> uint8_t *end)
> {
> const uint8_t *a = p + 4 - ((intptr_t)p & 3);
> @@ -318,15 +333,8 @@ static const AVRational avc_sample_aspect_ratio[17] = {
> { 2, 1 },
> };
>
> -static inline int get_ue_golomb(GetBitContext *gb) {
> - int i;
> - for (i = 0; i < 32 && !get_bits1(gb); i++)
> - ;
> - return get_bitsz(gb, i) + (1 << i) - 1;
> -}
> -
> static inline int get_se_golomb(GetBitContext *gb) {
> - int v = get_ue_golomb(gb) + 1;
> + unsigned v = get_ue_golomb(gb) + 1;
> int sign = -(v & 1);
> return ((v >> 1) ^ sign) - sign;
> }
I think it's best if we use the lavc golomb code instead. When i
suggested to re implement it here i wasn't aware it was shared with lavf
within the golomb_tab.c source file.
_______________________________________________
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".
