Message ID | 20200719161543.7886-1-steinar+ffmpeg@gunderson.no |
---|---|
State | Accepted |
Headers | show |
Series | [FFmpeg-devel] avcodec/put_bits: Fix LZW warning | expand |
Context | Check | Description |
---|---|---|
andriy/default | pending | |
andriy/make | success | Make finished |
andriy/make_fate | success | Make fate finished |
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 */
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; 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;