diff mbox

[FFmpeg-devel,V3] lavc/golomb: Fix UE golomb overwrite issue.

Message ID 4ca41ff5-6830-541a-51b4-aed1a888b1c5@gmail.com
State Superseded
Headers show

Commit Message

Jun Zhao June 8, 2017, 12:29 a.m. UTC
V3: Clean the code logic base on Michael's review.
V2: Add Add set_ue_golomb_long() to support 32bits UE golomb and update the unit test.
From 4de3e0c30af7bc1901562000eda018a6d6849292 Mon Sep 17 00:00:00 2001
From: Jun Zhao <jun.zhao@intel.com>
Date: Fri, 2 Jun 2017 15:05:49 +0800
Subject: [PATCH V3] lavc/golomb: Fix UE golomb overwrite issue.

put_bits just support write up to 31 bits, when write 32 bit in
put_bits, it's will overwrite the bit buffer, because the default
assert level is 0, the av_assert2(n <= 31 && value < (1U << n))
in put_bits can not be trigger runtime. Add set_ue_golomb_long()
to support 32bits UE golomb.

Signed-off-by: Jun Zhao <jun.zhao@intel.com>
---
 libavcodec/golomb.h       | 17 ++++++++++++++++-
 libavcodec/put_bits.h     | 35 +++++++++++++++++++++++++++++++++++
 libavcodec/tests/golomb.c | 19 +++++++++++++++++++
 3 files changed, 70 insertions(+), 1 deletion(-)

Comments

Mark Thompson June 8, 2017, 10:02 a.m. UTC | #1
On 08/06/17 01:29, Jun Zhao wrote:
> V3: Clean the code logic base on Michael's review.
> V2: Add Add set_ue_golomb_long() to support 32bits UE golomb and update the unit test.
> 
> From 4de3e0c30af7bc1901562000eda018a6d6849292 Mon Sep 17 00:00:00 2001
> From: Jun Zhao <jun.zhao@intel.com>
> Date: Fri, 2 Jun 2017 15:05:49 +0800
> Subject: [PATCH V3] lavc/golomb: Fix UE golomb overwrite issue.
> 
> put_bits just support write up to 31 bits, when write 32 bit in
> put_bits, it's will overwrite the bit buffer, because the default
> assert level is 0, the av_assert2(n <= 31 && value < (1U << n))
> in put_bits can not be trigger runtime. Add set_ue_golomb_long()
> to support 32bits UE golomb.
> 
> Signed-off-by: Jun Zhao <jun.zhao@intel.com>
> ---
>  libavcodec/golomb.h       | 17 ++++++++++++++++-
>  libavcodec/put_bits.h     | 35 +++++++++++++++++++++++++++++++++++
>  libavcodec/tests/golomb.c | 19 +++++++++++++++++++
>  3 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> index 0833aff468..cba4861b10 100644
> --- a/libavcodec/golomb.h
> +++ b/libavcodec/golomb.h
> @@ -458,7 +458,7 @@ static inline int get_te(GetBitContext *s, int r, char *file, const char *func,
>  #endif /* TRACE */
>  
>  /**
> - * write unsigned exp golomb code.
> + * write unsigned exp golomb code. 2^16-2 at most.
>   */
>  static inline void set_ue_golomb(PutBitContext *pb, int i)
>  {
> @@ -473,6 +473,21 @@ static inline void set_ue_golomb(PutBitContext *pb, int i)
>  }
>  
>  /**
> + * write unsigned exp golomb code. 2^32-2 at most.
> + */
> +static inline void set_ue_golomb_long(PutBitContext *pb, uint32_t i)
> +{
> +    av_assert2(i <= (0xffffffff - 2));

That's not 2^32 - 2.  Maybe use UINT32_MAX - 1?

> +
> +    if (i < 256)
> +        put_bits(pb, ff_ue_golomb_len[i], i + 1);
> +    else {
> +        int e = av_log2(i + 1);
> +        put_bits64(pb, 2 * e + 1, i + 1);
> +    }
> +}

Please don't add a new function, just extend the existing one.  set_ue_golomb() is not used in any place where minor ricing is of any value, and having another function will only be confusing over which to use (or, more likely, people will always use the long version in order to avoid thinking about it just like they do with get_ue_golomb() - cf. "grep get_ue_golomb libavcodec/hevc*").

> +
> +/**
>   * write truncated unsigned exp golomb code.
>   */
>  static inline void set_te_golomb(PutBitContext *pb, int i, int range)
> diff --git a/libavcodec/put_bits.h b/libavcodec/put_bits.h
> index 68ed391195..06f0ebbeba 100644
> --- a/libavcodec/put_bits.h
> +++ b/libavcodec/put_bits.h
> @@ -221,6 +221,41 @@ static void av_unused put_bits32(PutBitContext *s, uint32_t value)
>  }
>  
>  /**
> + * Write up to 64 bits into a bitstream.
> + */
> +static inline void put_bits64(PutBitContext *s, int n, uint64_t value)

put_bits32() writes exactly 32 bits, so it sounds like put_bits64() will write exactly 64 bits.  I'm not sure what the function should be called, but not that.

> +{
> +    av_assert2(n <= 64 && value < (1UL << n));

1UL is 32-bit on ILP32 platforms, so 1 << n is zero for n > 31 and the assert will always fail.

Maybe "av_assert2(n == 64 || (n < 64 && value < UINT64_C(1) << n))"?

> +
> +    if (n < 32)
> +        put_bits(s, n, value);
> +    else if (n == 32)
> +        put_bits32(s, value);
> +    else if (n < 64) {
> +        uint32_t lo = value & 0xffffffff;
> +        uint32_t hi = value >> 32;
> +#ifdef BITSTREAM_WRITER_LE
> +        put_bits32(s, lo);
> +        put_bits(s, n - 32, hi);
> +#else
> +        put_bits(s, n - 32, hi);
> +        put_bits32(s, lo);
> +#endif
> +    } else {
> +        uint32_t lo = value & 0xffffffff;
> +        uint32_t hi = value >> 32;
> +#ifdef BITSTREAM_WRITER_LE
> +        put_bits32(s, lo);
> +        put_bits32(s, hi);
> +#else
> +        put_bits32(s, hi);
> +        put_bits32(s, lo);
> +#endif
> +
> +    }
> +}
> +
> +/**
>   * Return the pointer to the byte where the bitstream writer will put
>   * the next bit.
>   */
> diff --git a/libavcodec/tests/golomb.c b/libavcodec/tests/golomb.c
> index 965367b7be..85b8a9390b 100644
> --- a/libavcodec/tests/golomb.c
> +++ b/libavcodec/tests/golomb.c
> @@ -21,6 +21,7 @@
>  #include <stdint.h>
>  #include <stdio.h>
>  
> +#include "libavutil/internal.h"
>  #include "libavutil/mem.h"
>  
>  #include "libavcodec/get_bits.h"
> @@ -76,6 +77,24 @@ int main(void)
>          }
>      }
>  
> +#define EXTEND_L(i) ((i) << 4 | (i) & 15)
> +    init_put_bits(&pb, temp, SIZE);
> +    for (i = 0; i < COUNT; i++)
> +        set_ue_golomb_long(&pb, EXTEND_L(i));
> +    flush_put_bits(&pb);
> +
> +    init_get_bits(&gb, temp, 8 * SIZE);
> +    for (i = 0; i < COUNT; i++) {
> +        int j, s = show_bits_long(&gb, 32);
> +
> +        j = get_ue_golomb_long(&gb);
> +        if (j != EXTEND_L(i)) {
> +            fprintf(stderr, "get_ue_golomb_long: expected %d, got %d. "
> +                    "bits: %8x\n", EXTEND_L(i), j, s);
> +            ret = 1;
> +        }
> +    }
> +
>      init_put_bits(&pb, temp, SIZE);
>      for (i = 0; i < COUNT; i++)
>          set_se_golomb(&pb, i - COUNT / 2);
> -- 
> 2.11.0
>
Michael Niedermayer June 8, 2017, 11:35 a.m. UTC | #2
On Thu, Jun 08, 2017 at 11:02:30AM +0100, Mark Thompson wrote:
> On 08/06/17 01:29, Jun Zhao wrote:
> > V3: Clean the code logic base on Michael's review.
> > V2: Add Add set_ue_golomb_long() to support 32bits UE golomb and update the unit test.
> > 
> > From 4de3e0c30af7bc1901562000eda018a6d6849292 Mon Sep 17 00:00:00 2001
> > From: Jun Zhao <jun.zhao@intel.com>
> > Date: Fri, 2 Jun 2017 15:05:49 +0800
> > Subject: [PATCH V3] lavc/golomb: Fix UE golomb overwrite issue.
> > 
> > put_bits just support write up to 31 bits, when write 32 bit in
> > put_bits, it's will overwrite the bit buffer, because the default
> > assert level is 0, the av_assert2(n <= 31 && value < (1U << n))
> > in put_bits can not be trigger runtime. Add set_ue_golomb_long()
> > to support 32bits UE golomb.
> > 
> > Signed-off-by: Jun Zhao <jun.zhao@intel.com>
[...]
> > +
> > +    if (i < 256)
> > +        put_bits(pb, ff_ue_golomb_len[i], i + 1);
> > +    else {
> > +        int e = av_log2(i + 1);
> > +        put_bits64(pb, 2 * e + 1, i + 1);
> > +    }
> > +}
> 
> Please don't add a new function, just extend the existing one.  set_ue_golomb() is not used in any place where minor ricing is of any value, and having another function will only be confusing over which to use (or, more likely, people will always use the long version in order to avoid thinking about it just like they do with get_ue_golomb() - cf. "grep get_ue_golomb libavcodec/hevc*").

No question its easier to do something random than to think about
the range allowed by the specification.
But when writing data, its neccessary to comply to the specification,
otherwise one creates invalid streams. So theres no way around thinking
about the range even with just one function.



> 
> > +
> > +/**
> >   * write truncated unsigned exp golomb code.
> >   */
> >  static inline void set_te_golomb(PutBitContext *pb, int i, int range)
> > diff --git a/libavcodec/put_bits.h b/libavcodec/put_bits.h
> > index 68ed391195..06f0ebbeba 100644
> > --- a/libavcodec/put_bits.h
> > +++ b/libavcodec/put_bits.h
> > @@ -221,6 +221,41 @@ static void av_unused put_bits32(PutBitContext *s, uint32_t value)
> >  }
> >  
> >  /**
> > + * Write up to 64 bits into a bitstream.
> > + */
> > +static inline void put_bits64(PutBitContext *s, int n, uint64_t value)
> 
> put_bits32() writes exactly 32 bits, so it sounds like put_bits64() will write exactly 64 bits.  I'm not sure what the function should be called, but not that.

If you name it different then please also rename get_bits64() in a
seperate patch before
[get_bits64 was originally called get_bits_longlong until it was
 renamed to get_bits64 in result of a merge i did. Looking back that
 renaming was a bad idea. longlong isnt a great name either though]


[...]
Mark Thompson June 8, 2017, 12:07 p.m. UTC | #3
On 08/06/17 12:35, Michael Niedermayer wrote:
> On Thu, Jun 08, 2017 at 11:02:30AM +0100, Mark Thompson wrote:
>> On 08/06/17 01:29, Jun Zhao wrote:
>>> V3: Clean the code logic base on Michael's review.
>>> V2: Add Add set_ue_golomb_long() to support 32bits UE golomb and update the unit test.
>>>
>>> From 4de3e0c30af7bc1901562000eda018a6d6849292 Mon Sep 17 00:00:00 2001
>>> From: Jun Zhao <jun.zhao@intel.com>
>>> Date: Fri, 2 Jun 2017 15:05:49 +0800
>>> Subject: [PATCH V3] lavc/golomb: Fix UE golomb overwrite issue.
>>>
>>> put_bits just support write up to 31 bits, when write 32 bit in
>>> put_bits, it's will overwrite the bit buffer, because the default
>>> assert level is 0, the av_assert2(n <= 31 && value < (1U << n))
>>> in put_bits can not be trigger runtime. Add set_ue_golomb_long()
>>> to support 32bits UE golomb.
>>>
>>> Signed-off-by: Jun Zhao <jun.zhao@intel.com>
> [...]
>>> +
>>> +    if (i < 256)
>>> +        put_bits(pb, ff_ue_golomb_len[i], i + 1);
>>> +    else {
>>> +        int e = av_log2(i + 1);
>>> +        put_bits64(pb, 2 * e + 1, i + 1);
>>> +    }
>>> +}
>>
>> Please don't add a new function, just extend the existing one.  set_ue_golomb() is not used in any place where minor ricing is of any value, and having another function will only be confusing over which to use (or, more likely, people will always use the long version in order to avoid thinking about it just like they do with get_ue_golomb() - cf. "grep get_ue_golomb libavcodec/hevc*").
> 
> No question its easier to do something random than to think about
> the range allowed by the specification.
> But when writing data, its neccessary to comply to the specification,
> otherwise one creates invalid streams. So theres no way around thinking
> about the range even with just one function.

I don't understand what you're trying to say here.  I'm suggesting that it would be better to extend the range of values that set_ue_golomb() supports, so that it can correctly write values up to 2^32-2.  Since this is the largest value H.264 and H.265 ever require, any use with those standards is known to be in-range and no further thought is needed.  It is possible that some other codec might want to use larger Exp-Golomb codes - if that happens then indeed more thought will be required when implementing that codec, but it shouldn't affect anyone else.

- Mark
Michael Niedermayer June 8, 2017, 6:03 p.m. UTC | #4
On Thu, Jun 08, 2017 at 01:07:40PM +0100, Mark Thompson wrote:
> On 08/06/17 12:35, Michael Niedermayer wrote:
> > On Thu, Jun 08, 2017 at 11:02:30AM +0100, Mark Thompson wrote:
> >> On 08/06/17 01:29, Jun Zhao wrote:
> >>> V3: Clean the code logic base on Michael's review.
> >>> V2: Add Add set_ue_golomb_long() to support 32bits UE golomb and update the unit test.
> >>>
> >>> From 4de3e0c30af7bc1901562000eda018a6d6849292 Mon Sep 17 00:00:00 2001
> >>> From: Jun Zhao <jun.zhao@intel.com>
> >>> Date: Fri, 2 Jun 2017 15:05:49 +0800
> >>> Subject: [PATCH V3] lavc/golomb: Fix UE golomb overwrite issue.
> >>>
> >>> put_bits just support write up to 31 bits, when write 32 bit in
> >>> put_bits, it's will overwrite the bit buffer, because the default
> >>> assert level is 0, the av_assert2(n <= 31 && value < (1U << n))
> >>> in put_bits can not be trigger runtime. Add set_ue_golomb_long()
> >>> to support 32bits UE golomb.
> >>>
> >>> Signed-off-by: Jun Zhao <jun.zhao@intel.com>
> > [...]
> >>> +
> >>> +    if (i < 256)
> >>> +        put_bits(pb, ff_ue_golomb_len[i], i + 1);
> >>> +    else {
> >>> +        int e = av_log2(i + 1);
> >>> +        put_bits64(pb, 2 * e + 1, i + 1);
> >>> +    }
> >>> +}
> >>
> >> Please don't add a new function, just extend the existing one.  set_ue_golomb() is not used in any place where minor ricing is of any value, and having another function will only be confusing over which to use (or, more likely, people will always use the long version in order to avoid thinking about it just like they do with get_ue_golomb() - cf. "grep get_ue_golomb libavcodec/hevc*").
> > 
> > No question its easier to do something random than to think about
> > the range allowed by the specification.
> > But when writing data, its neccessary to comply to the specification,
> > otherwise one creates invalid streams. So theres no way around thinking
> > about the range even with just one function.
> 
> I don't understand what you're trying to say here.

Iam saying that the argument you use implies that one has to write
code quite sloppy. (that is way sloppier than we normally do)

The author has to think about what is the valid range before writing
a value in the bitstream.
You said "... people will always use the long version in order to avoid thinking about it ..."
That really is not what they can do. They have to think about what
range is valid so as to make sure they do not create invalid bitstreams


> I'm suggesting that it would be better to extend the range of values that set_ue_golomb() supports, so that it can correctly write values up to 2^32-2.
> Since this is the largest value H.264 and H.265 ever require,

> any use with those standards is known to be in-range and no further thought is needed.

as said above, thought is needed when writing things according to a
specification, so as to make sure one does not violate the
specification

The 2 options suggested if iam not missing one are

1. one function which supports the whole range
2. two functions one optimized for a small range, one for the full
range

For both cases, the user of this function must check the spec and
range for EVERY use and ensure that the values written are not
violating the relevant specification.
We do NOT need more invalid files ...

In some cases explicit checks will be needed at some earlier point
to ensure no invalid values are generated which would later be written.
In some cases no checks are needed but one still has to make sure
that is so and nothing is missing ...

Seperate of this are speed and code size aspects

If theres just one function, should that one function be "inline" ?
should maybe the code writing larger values be in a never inline
function called from it ?

inlining all this code on every call even if never used is a waste
of cpu cache, memory and so forth.
people comlpained about error messages taking up space (which are
tiny in comparission to this)


> It is possible that some other codec might want to use larger Exp-Golomb codes - if that happens then indeed more thought will be required when implementing that codec, but it shouldn't affect anyone else.
Jun Zhao June 9, 2017, 1:25 a.m. UTC | #5
On 2017/6/8 18:02, Mark Thompson wrote:
> On 08/06/17 01:29, Jun Zhao wrote:
>> V3: Clean the code logic base on Michael's review.
>> V2: Add Add set_ue_golomb_long() to support 32bits UE golomb and update the unit test.
>>
>> From 4de3e0c30af7bc1901562000eda018a6d6849292 Mon Sep 17 00:00:00 2001
>> From: Jun Zhao <jun.zhao@intel.com>
>> Date: Fri, 2 Jun 2017 15:05:49 +0800
>> Subject: [PATCH V3] lavc/golomb: Fix UE golomb overwrite issue.
>>
>> put_bits just support write up to 31 bits, when write 32 bit in
>> put_bits, it's will overwrite the bit buffer, because the default
>> assert level is 0, the av_assert2(n <= 31 && value < (1U << n))
>> in put_bits can not be trigger runtime. Add set_ue_golomb_long()
>> to support 32bits UE golomb.
>>
>> Signed-off-by: Jun Zhao <jun.zhao@intel.com>
>> ---
>>  libavcodec/golomb.h       | 17 ++++++++++++++++-
>>  libavcodec/put_bits.h     | 35 +++++++++++++++++++++++++++++++++++
>>  libavcodec/tests/golomb.c | 19 +++++++++++++++++++
>>  3 files changed, 70 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
>> index 0833aff468..cba4861b10 100644
>> --- a/libavcodec/golomb.h
>> +++ b/libavcodec/golomb.h
>> @@ -458,7 +458,7 @@ static inline int get_te(GetBitContext *s, int r, char *file, const char *func,
>>  #endif /* TRACE */
>>  
>>  /**
>> - * write unsigned exp golomb code.
>> + * write unsigned exp golomb code. 2^16-2 at most.
>>   */
>>  static inline void set_ue_golomb(PutBitContext *pb, int i)
>>  {
>> @@ -473,6 +473,21 @@ static inline void set_ue_golomb(PutBitContext *pb, int i)
>>  }
>>  
>>  /**
>> + * write unsigned exp golomb code. 2^32-2 at most.
>> + */
>> +static inline void set_ue_golomb_long(PutBitContext *pb, uint32_t i)
>> +{
>> +    av_assert2(i <= (0xffffffff - 2));
> 
> That's not 2^32 - 2.  Maybe use UINT32_MAX - 1
Your are right, will fix in the next version.

>> +
>> +    if (i < 256)
>> +        put_bits(pb, ff_ue_golomb_len[i], i + 1);
>> +    else {
>> +        int e = av_log2(i + 1);
>> +        put_bits64(pb, 2 * e + 1, i + 1);
>> +    }
>> +}
> 
> Please don't add a new function, just extend the existing one.  set_ue_golomb() is not used in any place where minor ricing is of any value, and having another function will only be confusing over which to use (or, more likely, people will always use the long version in order to avoid thinking about it just like they do with get_ue_golomb() - cf. "grep get_ue_golomb libavcodec/hevc*").

We have get_ue_golomb/get_ue_golomb_long/get_ue_golomb_31/..., I think add new set_ue_golomb_long is Ok,
the caller need to make the decision base on the value range even set_ue_golomb_long can cover the set_ue_golomb.

> 
>> +
>> +/**
>>   * write truncated unsigned exp golomb code.
>>   */
>>  static inline void set_te_golomb(PutBitContext *pb, int i, int range)
>> diff --git a/libavcodec/put_bits.h b/libavcodec/put_bits.h
>> index 68ed391195..06f0ebbeba 100644
>> --- a/libavcodec/put_bits.h
>> +++ b/libavcodec/put_bits.h
>> @@ -221,6 +221,41 @@ static void av_unused put_bits32(PutBitContext *s, uint32_t value)
>>  }
>>  
>>  /**
>> + * Write up to 64 bits into a bitstream.
>> + */
>> +static inline void put_bits64(PutBitContext *s, int n, uint64_t value)
> 
> put_bits32() writes exactly 32 bits, so it sounds like put_bits64() will write exactly 64 bits.  I'm not sure what the function should be called, but not that.
> 

For the function name, I can't find other good name short time. :(
>> +{
>> +    av_assert2(n <= 64 && value < (1UL << n));
> 
> 1UL is 32-bit on ILP32 platforms, so 1 << n is zero for n > 31 and the assert will always fail.
> 
> Maybe "av_assert2(n == 64 || (n < 64 && value < UINT64_C(1) << n))"?
> 

Thanks again.

>> +
>> +    if (n < 32)
>> +        put_bits(s, n, value);
>> +    else if (n == 32)
>> +        put_bits32(s, value);
>> +    else if (n < 64) {
>> +        uint32_t lo = value & 0xffffffff;
>> +        uint32_t hi = value >> 32;
>> +#ifdef BITSTREAM_WRITER_LE
>> +        put_bits32(s, lo);
>> +        put_bits(s, n - 32, hi);
>> +#else
>> +        put_bits(s, n - 32, hi);
>> +        put_bits32(s, lo);
>> +#endif
>> +    } else {
>> +        uint32_t lo = value & 0xffffffff;
>> +        uint32_t hi = value >> 32;
>> +#ifdef BITSTREAM_WRITER_LE
>> +        put_bits32(s, lo);
>> +        put_bits32(s, hi);
>> +#else
>> +        put_bits32(s, hi);
>> +        put_bits32(s, lo);
>> +#endif
>> +
>> +    }
>> +}
>> +
>> +/**
>>   * Return the pointer to the byte where the bitstream writer will put
>>   * the next bit.
>>   */
>> diff --git a/libavcodec/tests/golomb.c b/libavcodec/tests/golomb.c
>> index 965367b7be..85b8a9390b 100644
>> --- a/libavcodec/tests/golomb.c
>> +++ b/libavcodec/tests/golomb.c
>> @@ -21,6 +21,7 @@
>>  #include <stdint.h>
>>  #include <stdio.h>
>>  
>> +#include "libavutil/internal.h"
>>  #include "libavutil/mem.h"
>>  
>>  #include "libavcodec/get_bits.h"
>> @@ -76,6 +77,24 @@ int main(void)
>>          }
>>      }
>>  
>> +#define EXTEND_L(i) ((i) << 4 | (i) & 15)
>> +    init_put_bits(&pb, temp, SIZE);
>> +    for (i = 0; i < COUNT; i++)
>> +        set_ue_golomb_long(&pb, EXTEND_L(i));
>> +    flush_put_bits(&pb);
>> +
>> +    init_get_bits(&gb, temp, 8 * SIZE);
>> +    for (i = 0; i < COUNT; i++) {
>> +        int j, s = show_bits_long(&gb, 32);
>> +
>> +        j = get_ue_golomb_long(&gb);
>> +        if (j != EXTEND_L(i)) {
>> +            fprintf(stderr, "get_ue_golomb_long: expected %d, got %d. "
>> +                    "bits: %8x\n", EXTEND_L(i), j, s);
>> +            ret = 1;
>> +        }
>> +    }
>> +
>>      init_put_bits(&pb, temp, SIZE);
>>      for (i = 0; i < COUNT; i++)
>>          set_se_golomb(&pb, i - COUNT / 2);
>> -- 
>> 2.11.0
>>
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
diff mbox

Patch

diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
index 0833aff468..cba4861b10 100644
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -458,7 +458,7 @@  static inline int get_te(GetBitContext *s, int r, char *file, const char *func,
 #endif /* TRACE */
 
 /**
- * write unsigned exp golomb code.
+ * write unsigned exp golomb code. 2^16-2 at most.
  */
 static inline void set_ue_golomb(PutBitContext *pb, int i)
 {
@@ -473,6 +473,21 @@  static inline void set_ue_golomb(PutBitContext *pb, int i)
 }
 
 /**
+ * write unsigned exp golomb code. 2^32-2 at most.
+ */
+static inline void set_ue_golomb_long(PutBitContext *pb, uint32_t i)
+{
+    av_assert2(i <= (0xffffffff - 2));
+
+    if (i < 256)
+        put_bits(pb, ff_ue_golomb_len[i], i + 1);
+    else {
+        int e = av_log2(i + 1);
+        put_bits64(pb, 2 * e + 1, i + 1);
+    }
+}
+
+/**
  * write truncated unsigned exp golomb code.
  */
 static inline void set_te_golomb(PutBitContext *pb, int i, int range)
diff --git a/libavcodec/put_bits.h b/libavcodec/put_bits.h
index 68ed391195..06f0ebbeba 100644
--- a/libavcodec/put_bits.h
+++ b/libavcodec/put_bits.h
@@ -221,6 +221,41 @@  static void av_unused put_bits32(PutBitContext *s, uint32_t value)
 }
 
 /**
+ * Write up to 64 bits into a bitstream.
+ */
+static inline void put_bits64(PutBitContext *s, int n, uint64_t value)
+{
+    av_assert2(n <= 64 && value < (1UL << n));
+
+    if (n < 32)
+        put_bits(s, n, value);
+    else if (n == 32)
+        put_bits32(s, value);
+    else if (n < 64) {
+        uint32_t lo = value & 0xffffffff;
+        uint32_t hi = value >> 32;
+#ifdef BITSTREAM_WRITER_LE
+        put_bits32(s, lo);
+        put_bits(s, n - 32, hi);
+#else
+        put_bits(s, n - 32, hi);
+        put_bits32(s, lo);
+#endif
+    } else {
+        uint32_t lo = value & 0xffffffff;
+        uint32_t hi = value >> 32;
+#ifdef BITSTREAM_WRITER_LE
+        put_bits32(s, lo);
+        put_bits32(s, hi);
+#else
+        put_bits32(s, hi);
+        put_bits32(s, lo);
+#endif
+
+    }
+}
+
+/**
  * Return the pointer to the byte where the bitstream writer will put
  * the next bit.
  */
diff --git a/libavcodec/tests/golomb.c b/libavcodec/tests/golomb.c
index 965367b7be..85b8a9390b 100644
--- a/libavcodec/tests/golomb.c
+++ b/libavcodec/tests/golomb.c
@@ -21,6 +21,7 @@ 
 #include <stdint.h>
 #include <stdio.h>
 
+#include "libavutil/internal.h"
 #include "libavutil/mem.h"
 
 #include "libavcodec/get_bits.h"
@@ -76,6 +77,24 @@  int main(void)
         }
     }
 
+#define EXTEND_L(i) ((i) << 4 | (i) & 15)
+    init_put_bits(&pb, temp, SIZE);
+    for (i = 0; i < COUNT; i++)
+        set_ue_golomb_long(&pb, EXTEND_L(i));
+    flush_put_bits(&pb);
+
+    init_get_bits(&gb, temp, 8 * SIZE);
+    for (i = 0; i < COUNT; i++) {
+        int j, s = show_bits_long(&gb, 32);
+
+        j = get_ue_golomb_long(&gb);
+        if (j != EXTEND_L(i)) {
+            fprintf(stderr, "get_ue_golomb_long: expected %d, got %d. "
+                    "bits: %8x\n", EXTEND_L(i), j, s);
+            ret = 1;
+        }
+    }
+
     init_put_bits(&pb, temp, SIZE);
     for (i = 0; i < COUNT; i++)
         set_se_golomb(&pb, i - COUNT / 2);