[FFmpeg-devel] [PATCH v6] avcodec: add farbfeld encoder
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Wed Jun 5 13:02:08 EEST 2024
Stefano Sabatini:
> On date Tuesday 2024-06-04 17:28:35 -0500, Marcus B Spencer wrote:
>> farbfeld is an uncompressed image format that is a part of suckless
>> tools (https://tools.suckless.org).
>>
>> Its documentation is available at https://tools.suckless.org/farbfeld.
>>
>> Add support for this image format in avcodec and update the image2
>> format accordingly.
>>
>> Signed-off-by: Marcus B Spencer <marcus at marcusspencer.xyz>
>> ---
>> Changelog | 1 +
>> doc/general_contents.texi | 2 +
>> libavcodec/Makefile | 1 +
>> libavcodec/allcodecs.c | 1 +
>> libavcodec/codec_desc.c | 7 ++++
>> libavcodec/codec_id.h | 1 +
>> libavcodec/farbfeldenc.c | 84 +++++++++++++++++++++++++++++++++++++++
>> libavcodec/version.h | 2 +-
>> libavformat/img2.c | 1 +
>> libavformat/img2enc.c | 2 +-
>> 10 files changed, 100 insertions(+), 2 deletions(-)
>> create mode 100644 libavcodec/farbfeldenc.c
>>
>> diff --git a/Changelog b/Changelog
>> index 03d6b29ad8..d2b831c623 100644
>> --- a/Changelog
>> +++ b/Changelog
>> @@ -12,6 +12,7 @@ version <next>:
>> - qsv_params option added for QSV encoders
>> - VVC decoder compatible with DVB test content
>> - xHE-AAC decoder
>> +- farbfeld encoder
>>
>>
>> version 7.0:
>> diff --git a/doc/general_contents.texi b/doc/general_contents.texi
>> index e7cf4f8239..129a325ecf 100644
>> --- a/doc/general_contents.texi
>> +++ b/doc/general_contents.texi
>> @@ -853,6 +853,8 @@ following image formats are supported:
>> @tab X PixMap image format
>> @item XWD @tab X @tab X
>> @tab X Window Dump image format
>> + at item FF @tab X @tab
>> + @tab farbfeld uncompressed image format
>> @end multitable
>>
>> @code{X} means that the feature in that column (encoding / decoding) is supported.
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index 8ab4398b6c..9647279423 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -356,6 +356,7 @@ OBJS-$(CONFIG_ESCAPE130_DECODER) += escape130.o
>> OBJS-$(CONFIG_EVRC_DECODER) += evrcdec.o acelp_vectors.o lsp.o
>> OBJS-$(CONFIG_EXR_DECODER) += exr.o exrdsp.o half2float.o
>> OBJS-$(CONFIG_EXR_ENCODER) += exrenc.o float2half.o
>> +OBJS-$(CONFIG_FARBFELD_ENCODER) += farbfeldenc.o
>> OBJS-$(CONFIG_FASTAUDIO_DECODER) += fastaudio.o
>> OBJS-$(CONFIG_FFV1_DECODER) += ffv1dec.o ffv1.o
>> OBJS-$(CONFIG_FFV1_ENCODER) += ffv1enc.o ffv1.o
>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> index b102a8069e..b0ebb72396 100644
>> --- a/libavcodec/allcodecs.c
>> +++ b/libavcodec/allcodecs.c
>> @@ -115,6 +115,7 @@ extern const FFCodec ff_escape124_decoder;
>> extern const FFCodec ff_escape130_decoder;
>> extern const FFCodec ff_exr_encoder;
>> extern const FFCodec ff_exr_decoder;
>> +extern const FFCodec ff_farbfeld_encoder;
>> extern const FFCodec ff_ffv1_encoder;
>> extern const FFCodec ff_ffv1_decoder;
>> extern const FFCodec ff_ffvhuff_encoder;
>> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
>> index a28ef68061..33dbd2ce94 100644
>> --- a/libavcodec/codec_desc.c
>> +++ b/libavcodec/codec_desc.c
>> @@ -1959,6 +1959,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
>> .long_name = NULL_IF_CONFIG_SMALL("LEAD MCMP"),
>> .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
>> },
>> + {
>> + .id = AV_CODEC_ID_FARBFELD,
>> + .type = AVMEDIA_TYPE_VIDEO,
>> + .name = "farbfeld",
>> + .long_name = NULL_IF_CONFIG_SMALL("farbfeld uncompressed image"),
>> + .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
>> + },
>>
>> /* various PCM "codecs" */
>> {
>> diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h
>> index 0ab1e34a61..d4b0d23f7e 100644
>> --- a/libavcodec/codec_id.h
>> +++ b/libavcodec/codec_id.h
>> @@ -322,6 +322,7 @@ enum AVCodecID {
>> AV_CODEC_ID_RTV1,
>> AV_CODEC_ID_VMIX,
>> AV_CODEC_ID_LEAD,
>> + AV_CODEC_ID_FARBFELD,
>>
>> /* various PCM "codecs" */
>> AV_CODEC_ID_FIRST_AUDIO = 0x10000, ///< A dummy id pointing at the start of audio codecs
>> diff --git a/libavcodec/farbfeldenc.c b/libavcodec/farbfeldenc.c
>> new file mode 100644
>> index 0000000000..686ba5665e
>> --- /dev/null
>> +++ b/libavcodec/farbfeldenc.c
>> @@ -0,0 +1,84 @@
>> +/*
>> + * Copyright (c) 2024 Marcus B Spencer <marcus at marcusspencer.xyz>
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the “Software”), to
>> + * deal in the Software without restriction, including without limitation the
>> + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
>> + * sell copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + */
>> +
>> +#include "avcodec.h"
>> +#include "bytestream.h"
>> +#include "codec_internal.h"
>> +#include "encode.h"
>> +#include "libavutil/imgutils.h"
>> +
>> +#define HEADER_SIZE 16
>> +
>> +static int farbfeld_encode_frame(AVCodecContext *ctx, AVPacket *pkt,
>> + const AVFrame *p, int *got_packet)
>> +{
>
>> + int raw_img_size = av_image_get_buffer_size(
>> + p->format,
>> + p->width,
>> + p->height,
>> + 1
>> + );
>
>> + int64_t buf_size = (int64_t)raw_img_size + HEADER_SIZE;
>
> not yet, this might change the sign for a negative raw_img_size, you
> need two separate checks (one is not enough), as in the following:
>
> int raw_img_size = av_image_get_buffer_size(p->format, p->width,p->height, 1);
>
> if (raw_image_size < 0)
> return raw_image_size;
>
> int buf_size = raw_img_size + HEADER_SIZE;
> if (buf_size < 0)
> return AVERROR(EINVAL);
This is absolutely wrong: raw_img_size is nonnegative here as is
HEADER_SIZE and the addition will be performed as an int, so that
overflow would be UB which implies that the compiler can optimize this
check away.
One does not need two checks as long as int is 32 bits (because then one
can just perform the addition in 64bits). Just use the following (#if
has been used because compilers have a tendency to emit warnings if a
particular check is tautologically false):
#if INT_MAX > INT64_MAX - HEADER_SIZE
if (raw_img_size > INT64_MAX - HEADER_SIZE)
return AVERROR(ERANGE);
#endif
>
> ...
>
> Looks good otherwise, thanks.
>
> BTW, for proper integration would be good to also add a decoder (which
> should also be simple).
More information about the ffmpeg-devel
mailing list