Message ID | 20230608142029.16564-2-thilo.borgmann@mail.de |
---|---|
State | New |
Headers | show |
Series | webp: add support for animated WebP decoding | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | success | Make fate finished |
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
On Thu, Jun 8, 2023 at 7:20 AM Thilo Borgmann <thilo.borgmann@mail.de> wrote: > > --- > libavcodec/webp.c | 17 +-------------- > libavcodec/webp.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 56 insertions(+), 16 deletions(-) > create mode 100644 libavcodec/webp.h > lgtm.
Thilo Borgmann: > --- > libavcodec/webp.c | 17 +-------------- > libavcodec/webp.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 56 insertions(+), 16 deletions(-) > create mode 100644 libavcodec/webp.h > > diff --git a/libavcodec/webp.c b/libavcodec/webp.c > index d35cb66f8d..15152ec8fb 100644 > --- a/libavcodec/webp.c > +++ b/libavcodec/webp.c > @@ -52,22 +52,7 @@ > #include "thread.h" > #include "tiff_common.h" > #include "vp8.h" > - > -#define VP8X_FLAG_ANIMATION 0x02 > -#define VP8X_FLAG_XMP_METADATA 0x04 > -#define VP8X_FLAG_EXIF_METADATA 0x08 > -#define VP8X_FLAG_ALPHA 0x10 > -#define VP8X_FLAG_ICC 0x20 > - > -#define MAX_PALETTE_SIZE 256 > -#define MAX_CACHE_BITS 11 > -#define NUM_CODE_LENGTH_CODES 19 > -#define HUFFMAN_CODES_PER_META_CODE 5 > -#define NUM_LITERAL_CODES 256 > -#define NUM_LENGTH_CODES 24 > -#define NUM_DISTANCE_CODES 40 > -#define NUM_SHORT_DISTANCES 120 > -#define MAX_HUFFMAN_CODE_LENGTH 15 > +#include "webp.h" > > static const uint16_t alphabet_sizes[HUFFMAN_CODES_PER_META_CODE] = { > NUM_LITERAL_CODES + NUM_LENGTH_CODES, > diff --git a/libavcodec/webp.h b/libavcodec/webp.h > new file mode 100644 > index 0000000000..90baa71182 > --- /dev/null > +++ b/libavcodec/webp.h > @@ -0,0 +1,55 @@ > +/* > + * WebP image format definitions > + * Copyright (c) 2020 Pexeso Inc. > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +/** > + * @file > + * WebP image format definitions. > + */ > + > +#ifndef AVCODEC_WEBP_H > +#define AVCODEC_WEBP_H > + > +#define VP8X_FLAG_ANIMATION 0x02 > +#define VP8X_FLAG_XMP_METADATA 0x04 > +#define VP8X_FLAG_EXIF_METADATA 0x08 > +#define VP8X_FLAG_ALPHA 0x10 > +#define VP8X_FLAG_ICC 0x20 > + > +#define ANMF_DISPOSAL_METHOD 0x01 > +#define ANMF_DISPOSAL_METHOD_UNCHANGED 0x00 > +#define ANMF_DISPOSAL_METHOD_BACKGROUND 0x01 > + > +#define ANMF_BLENDING_METHOD 0x02 > +#define ANMF_BLENDING_METHOD_ALPHA 0x00 > +#define ANMF_BLENDING_METHOD_OVERWRITE 0x02 > + > +#define MAX_PALETTE_SIZE 256 > +#define MAX_CACHE_BITS 11 > +#define NUM_CODE_LENGTH_CODES 19 > +#define HUFFMAN_CODES_PER_META_CODE 5 > +#define NUM_LITERAL_CODES 256 > +#define NUM_LENGTH_CODES 24 > +#define NUM_DISTANCE_CODES 40 > +#define NUM_SHORT_DISTANCES 120 > +#define MAX_HUFFMAN_CODE_LENGTH 15 > + > + > +#endif /* AVCODEC_WEBP_H */ 1. Some of these defines (like MAX_CACHE_BITS) are unused now and seem to stay that way in your patchset. 2. If you move defines in a header, you need to ensure that they are properly prefixed so that no conflicts can arise. This is particularly true of defines like VP8X_FLAG_* whose name actually indicates that they belong into a vp8.h. 3. It seems that your patchset only includes this header in webp.c; they are not used outside of it. So there is no need for a header. - Andreas
Am 14.06.23 um 12:08 schrieb Andreas Rheinhardt: > Thilo Borgmann: >> --- >> libavcodec/webp.c | 17 +-------------- >> libavcodec/webp.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 56 insertions(+), 16 deletions(-) >> create mode 100644 libavcodec/webp.h >> >> diff --git a/libavcodec/webp.c b/libavcodec/webp.c >> index d35cb66f8d..15152ec8fb 100644 >> --- a/libavcodec/webp.c >> +++ b/libavcodec/webp.c >> @@ -52,22 +52,7 @@ >> #include "thread.h" >> #include "tiff_common.h" >> #include "vp8.h" >> - >> -#define VP8X_FLAG_ANIMATION 0x02 >> -#define VP8X_FLAG_XMP_METADATA 0x04 >> -#define VP8X_FLAG_EXIF_METADATA 0x08 >> -#define VP8X_FLAG_ALPHA 0x10 >> -#define VP8X_FLAG_ICC 0x20 >> - >> -#define MAX_PALETTE_SIZE 256 >> -#define MAX_CACHE_BITS 11 >> -#define NUM_CODE_LENGTH_CODES 19 >> -#define HUFFMAN_CODES_PER_META_CODE 5 >> -#define NUM_LITERAL_CODES 256 >> -#define NUM_LENGTH_CODES 24 >> -#define NUM_DISTANCE_CODES 40 >> -#define NUM_SHORT_DISTANCES 120 >> -#define MAX_HUFFMAN_CODE_LENGTH 15 >> +#include "webp.h" >> >> static const uint16_t alphabet_sizes[HUFFMAN_CODES_PER_META_CODE] = { >> NUM_LITERAL_CODES + NUM_LENGTH_CODES, >> diff --git a/libavcodec/webp.h b/libavcodec/webp.h >> new file mode 100644 >> index 0000000000..90baa71182 >> --- /dev/null >> +++ b/libavcodec/webp.h >> @@ -0,0 +1,55 @@ >> +/* >> + * WebP image format definitions >> + * Copyright (c) 2020 Pexeso Inc. >> + * >> + * This file is part of FFmpeg. >> + * >> + * FFmpeg is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU Lesser General Public >> + * License as published by the Free Software Foundation; either >> + * version 2.1 of the License, or (at your option) any later version. >> + * >> + * FFmpeg is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * Lesser General Public License for more details. >> + * >> + * You should have received a copy of the GNU Lesser General Public >> + * License along with FFmpeg; if not, write to the Free Software >> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA >> + */ >> + >> +/** >> + * @file >> + * WebP image format definitions. >> + */ >> + >> +#ifndef AVCODEC_WEBP_H >> +#define AVCODEC_WEBP_H >> + >> +#define VP8X_FLAG_ANIMATION 0x02 >> +#define VP8X_FLAG_XMP_METADATA 0x04 >> +#define VP8X_FLAG_EXIF_METADATA 0x08 >> +#define VP8X_FLAG_ALPHA 0x10 >> +#define VP8X_FLAG_ICC 0x20 >> + >> +#define ANMF_DISPOSAL_METHOD 0x01 >> +#define ANMF_DISPOSAL_METHOD_UNCHANGED 0x00 >> +#define ANMF_DISPOSAL_METHOD_BACKGROUND 0x01 >> + >> +#define ANMF_BLENDING_METHOD 0x02 >> +#define ANMF_BLENDING_METHOD_ALPHA 0x00 >> +#define ANMF_BLENDING_METHOD_OVERWRITE 0x02 >> + >> +#define MAX_PALETTE_SIZE 256 >> +#define MAX_CACHE_BITS 11 >> +#define NUM_CODE_LENGTH_CODES 19 >> +#define HUFFMAN_CODES_PER_META_CODE 5 >> +#define NUM_LITERAL_CODES 256 >> +#define NUM_LENGTH_CODES 24 >> +#define NUM_DISTANCE_CODES 40 >> +#define NUM_SHORT_DISTANCES 120 >> +#define MAX_HUFFMAN_CODE_LENGTH 15 >> + >> + >> +#endif /* AVCODEC_WEBP_H */ > > 1. Some of these defines (like MAX_CACHE_BITS) are unused now and seem > to stay that way in your patchset. > 2. If you move defines in a header, you need to ensure that they are > properly prefixed so that no conflicts can arise. This is particularly > true of defines like VP8X_FLAG_* whose name actually indicates that they > belong into a vp8.h. > 3. It seems that your patchset only includes this header in webp.c; they > are not used outside of it. So there is no need for a header. All true, I believe. Having it in a header originates from the original patchset including a parser where the header is then used as well. Because the parser did not get any feedback I posted only the decoder here and kept the header file anyway to keep the decoder patch smaller. I can keep it in there if you wish, though - then the need for prefixing becomes void, IIUC. I think it makes no sense to drap these defines into vp8.h as I believe they make sense only in the VP8-in-WebP case. v2 will include it in the decoder then, thx! -Thilo
diff --git a/libavcodec/webp.c b/libavcodec/webp.c index d35cb66f8d..15152ec8fb 100644 --- a/libavcodec/webp.c +++ b/libavcodec/webp.c @@ -52,22 +52,7 @@ #include "thread.h" #include "tiff_common.h" #include "vp8.h" - -#define VP8X_FLAG_ANIMATION 0x02 -#define VP8X_FLAG_XMP_METADATA 0x04 -#define VP8X_FLAG_EXIF_METADATA 0x08 -#define VP8X_FLAG_ALPHA 0x10 -#define VP8X_FLAG_ICC 0x20 - -#define MAX_PALETTE_SIZE 256 -#define MAX_CACHE_BITS 11 -#define NUM_CODE_LENGTH_CODES 19 -#define HUFFMAN_CODES_PER_META_CODE 5 -#define NUM_LITERAL_CODES 256 -#define NUM_LENGTH_CODES 24 -#define NUM_DISTANCE_CODES 40 -#define NUM_SHORT_DISTANCES 120 -#define MAX_HUFFMAN_CODE_LENGTH 15 +#include "webp.h" static const uint16_t alphabet_sizes[HUFFMAN_CODES_PER_META_CODE] = { NUM_LITERAL_CODES + NUM_LENGTH_CODES, diff --git a/libavcodec/webp.h b/libavcodec/webp.h new file mode 100644 index 0000000000..90baa71182 --- /dev/null +++ b/libavcodec/webp.h @@ -0,0 +1,55 @@ +/* + * WebP image format definitions + * Copyright (c) 2020 Pexeso Inc. + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/** + * @file + * WebP image format definitions. + */ + +#ifndef AVCODEC_WEBP_H +#define AVCODEC_WEBP_H + +#define VP8X_FLAG_ANIMATION 0x02 +#define VP8X_FLAG_XMP_METADATA 0x04 +#define VP8X_FLAG_EXIF_METADATA 0x08 +#define VP8X_FLAG_ALPHA 0x10 +#define VP8X_FLAG_ICC 0x20 + +#define ANMF_DISPOSAL_METHOD 0x01 +#define ANMF_DISPOSAL_METHOD_UNCHANGED 0x00 +#define ANMF_DISPOSAL_METHOD_BACKGROUND 0x01 + +#define ANMF_BLENDING_METHOD 0x02 +#define ANMF_BLENDING_METHOD_ALPHA 0x00 +#define ANMF_BLENDING_METHOD_OVERWRITE 0x02 + +#define MAX_PALETTE_SIZE 256 +#define MAX_CACHE_BITS 11 +#define NUM_CODE_LENGTH_CODES 19 +#define HUFFMAN_CODES_PER_META_CODE 5 +#define NUM_LITERAL_CODES 256 +#define NUM_LENGTH_CODES 24 +#define NUM_DISTANCE_CODES 40 +#define NUM_SHORT_DISTANCES 120 +#define MAX_HUFFMAN_CODE_LENGTH 15 + + +#endif /* AVCODEC_WEBP_H */