[FFmpeg-devel] avcodec/put_bits: Fix LZW warning

lzwenc stores a function pointer to either put_bits or put_bits_le.
Update the function pointer's prototype after the recent change.
---
 libavcodec/lzw.h    | 4 +++-
 libavcodec/lzwenc.c | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

On Sun, Jul 19, 2020 at 6:15 PM Steinar H. Gunderson
<steinar+ffmpeg@gunderson.no> wrote:
>
> lzwenc stores a function pointer to either put_bits or put_bits_le.
> Update the function pointer's prototype after the recent change.
> ---
>  libavcodec/lzw.h    | 4 +++-
>  libavcodec/lzwenc.c | 4 ++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/lzw.h b/libavcodec/lzw.h
> index 6af8a6b83a..400a479809 100644
> --- a/libavcodec/lzw.h
> +++ b/libavcodec/lzw.h
> @@ -32,6 +32,8 @@
>
>  #include <stdint.h>
>
> +#include "put_bits.h"
> +
>  struct PutBitContext;

This forward declaration is probably not needed anymore once you
include put_bits.h.

I do wonder if there was a concern with including the file before that
caused the original author to use the forward declaration though...

>
>  enum FF_LZW_MODES{
> @@ -55,7 +57,7 @@ extern const int ff_lzw_encode_state_size;
>
>  void ff_lzw_encode_init(struct LZWEncodeState *s, uint8_t *outbuf, int outsize,
>                          int maxbits, enum FF_LZW_MODES mode,
> -                        void (*lzw_put_bits)(struct PutBitContext *, int, unsigned int));
> +                        void (*lzw_put_bits)(struct PutBitContext *, int, BitBuf));
>  int ff_lzw_encode(struct LZWEncodeState * s, const uint8_t * inbuf, int insize);
>  int ff_lzw_encode_flush(struct LZWEncodeState *s,
>                          void (*lzw_flush_put_bits)(struct PutBitContext *));
> diff --git a/libavcodec/lzwenc.c b/libavcodec/lzwenc.c
> index 03080ee587..c1b96905e8 100644
> --- a/libavcodec/lzwenc.c
> +++ b/libavcodec/lzwenc.c
> @@ -60,7 +60,7 @@ typedef struct LZWEncodeState {
>      int output_bytes;        ///< Number of written bytes
>      int last_code;           ///< Value of last output code or LZW_PREFIX_EMPTY
>      enum FF_LZW_MODES mode;  ///< TIFF or GIF
> -    void (*put_bits)(PutBitContext *, int, unsigned); ///< GIF is LE while TIFF is BE
> +    void (*put_bits)(PutBitContext *, int, BitBuf); ///< GIF is LE while TIFF is BE
>  }LZWEncodeState;
>
>
> @@ -201,7 +201,7 @@ static int writtenBytes(LZWEncodeState *s){
>   */
>  void ff_lzw_encode_init(LZWEncodeState *s, uint8_t *outbuf, int outsize,
>                          int maxbits, enum FF_LZW_MODES mode,
> -                        void (*lzw_put_bits)(PutBitContext *, int, unsigned))
> +                        void (*lzw_put_bits)(PutBitContext *, int, BitBuf))
>  {
>      s->clear_code = 256;
>      s->end_code = 257;
> --
> 2.20.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Hendrik Leppkes:
> On Sun, Jul 19, 2020 at 6:15 PM Steinar H. Gunderson
> <steinar+ffmpeg@gunderson.no> wrote:
>>
>> lzwenc stores a function pointer to either put_bits or put_bits_le.
>> Update the function pointer's prototype after the recent change.
>> ---
>>  libavcodec/lzw.h    | 4 +++-
>>  libavcodec/lzwenc.c | 4 ++--
>>  2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/lzw.h b/libavcodec/lzw.h
>> index 6af8a6b83a..400a479809 100644
>> --- a/libavcodec/lzw.h
>> +++ b/libavcodec/lzw.h
>> @@ -32,6 +32,8 @@
>>
>>  #include <stdint.h>
>>
>> +#include "put_bits.h"
>> +
>>  struct PutBitContext;
> 
> This forward declaration is probably not needed anymore once you
> include put_bits.h.
> 
> I do wonder if there was a concern with including the file before that
> caused the original author to use the forward declaration though...
> 

The reason is probably that gif and tiff (the two users of this) use
different endianness (gif uses LE). A scenario like

#include "lzw.h"
#define BITSTREAM_WRITER_LE
#include "put_bits.h"

would be silently broken by this patch (the bitstream writer would be
big-endian).

Notice that gif.c defines BITSTREAM_WRITER_LE before including anything.

- Andreas

On Sun, Jul 19, 2020 at 07:48:47PM +0200, Andreas Rheinhardt wrote:
> The reason is probably that gif and tiff (the two users of this) use
> different endianness (gif uses LE). A scenario like
> 
> #include "lzw.h"
> #define BITSTREAM_WRITER_LE
> #include "put_bits.h"
> 
> would be silently broken by this patch (the bitstream writer would be
> big-endian).
> 
> Notice that gif.c defines BITSTREAM_WRITER_LE before including anything.

Yes, it's a bit tricky. I'm not convinced having a header silently behave
differently depending on a macro being set is a good idea... Perhaps I
should simply take out BITSTREAM_WRITER_LE from gif.c and replace it with
using put_bits_le?

/* Steinar */