diff mbox series

[FFmpeg-devel,v6] avcodec: add farbfeld encoder

Message ID 20240604222835.166462-1-marcus@marcusspencer.xyz
State New
Headers show
Series [FFmpeg-devel,v6] avcodec: add farbfeld encoder | expand

Checks

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

Commit Message

Marcus B Spencer June 4, 2024, 10:28 p.m. UTC
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@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

Comments

Stefano Sabatini June 5, 2024, 9:51 a.m. UTC | #1
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@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
> +@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@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);

...

Looks good otherwise, thanks.

BTW, for proper integration would be good to also add a decoder (which
should also be simple).
Andreas Rheinhardt June 5, 2024, 10:02 a.m. UTC | #2
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@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
>> +@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@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).
Stefano Sabatini June 5, 2024, 11 a.m. UTC | #3
On date Wednesday 2024-06-05 12:02:08 +0200, Andreas Rheinhardt wrote:
> Stefano Sabatini:
> > On date Tuesday 2024-06-04 17:28:35 -0500, Marcus B Spencer wrote:
[...]
> >> +#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.

Correct, the following should avoid the UB if I'm not mistaken again:

if (HEADER_SIZE > (INT_MAX - raw_img_size))
     return AVERROR(EINVAL);
int buf_size = raw_img_size + HEADER_SIZE;
...

> One does not need two checks as long as int is 32 bits (because then one
> can just perform the addition in 64bits).

sizeof(int) is not defined by the C standard, so you cannot assume it
is 32 bits (even if on most platforms/compilers it will be)

> 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
Andreas Rheinhardt June 5, 2024, 11:14 a.m. UTC | #4
Stefano Sabatini:
> On date Wednesday 2024-06-05 12:02:08 +0200, Andreas Rheinhardt wrote:
>> Stefano Sabatini:
>>> On date Tuesday 2024-06-04 17:28:35 -0500, Marcus B Spencer wrote:
> [...]
>>>> +#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.
> 
> Correct, the following should avoid the UB if I'm not mistaken again:
> 
> if (HEADER_SIZE > (INT_MAX - raw_img_size))
>      return AVERROR(EINVAL);
> int buf_size = raw_img_size + HEADER_SIZE;
> ...
> 
>> One does not need two checks as long as int is 32 bits (because then one
>> can just perform the addition in 64bits).
> 
> sizeof(int) is not defined by the C standard, so you cannot assume it
> is 32 bits (even if on most platforms/compilers it will be)
> 

Did you even read the following? It handles the case where simply using
64bits is not enough.

>> 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
Stefano Sabatini June 5, 2024, 1:21 p.m. UTC | #5
On date Wednesday 2024-06-05 13:14:01 +0200, Andreas Rheinhardt wrote:
> Stefano Sabatini:
[...]
> >> One does not need two checks as long as int is 32 bits (because then one
> >> can just perform the addition in 64bits).
> > 
> > sizeof(int) is not defined by the C standard, so you cannot assume it
> > is 32 bits (even if on most platforms/compilers it will be)
> > 
> 

> Did you even read the following? It handles the case where simply using
> 64bits is not enough.

Yes, but there is no need to mix types, introduce ifdeffery and make
more assumptions when there is a simpler solution, please let's stick
at that.
 
> >> 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
Andreas Rheinhardt June 5, 2024, 1:22 p.m. UTC | #6
Stefano Sabatini:
> On date Wednesday 2024-06-05 13:14:01 +0200, Andreas Rheinhardt wrote:
>> Stefano Sabatini:
> [...]
>>>> One does not need two checks as long as int is 32 bits (because then one
>>>> can just perform the addition in 64bits).
>>>
>>> sizeof(int) is not defined by the C standard, so you cannot assume it
>>> is 32 bits (even if on most platforms/compilers it will be)
>>>
>>
> 
>> Did you even read the following? It handles the case where simply using
>> 64bits is not enough.
> 
> Yes, but there is no need to mix types, introduce ifdeffery and make
> more assumptions when there is a simpler solution, please let's stick
> at that.
>  

Your "simpler solution" adds a (mostly) avoidable branch.

>>>> 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
diff mbox series

Patch

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
+@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@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;
+    uint8_t *buf;
+    int ret;
+
+    if (raw_img_size < 0)
+        return raw_img_size;
+
+    if ((ret = ff_get_encode_buffer(ctx, pkt, buf_size, 0)) < 0)
+        return ret;
+
+    buf = pkt->data;
+
+    bytestream_put_buffer(&buf, "farbfeld", 8);
+
+    bytestream_put_be32(&buf, ctx->width);
+    bytestream_put_be32(&buf, ctx->height);
+
+    av_image_copy_to_buffer(
+        buf,
+        raw_img_size,
+        (const uint8_t *const *)p->data,
+        p->linesize,
+        p->format,
+        p->width,
+        p->height,
+        1
+    );
+
+    *got_packet = 1;
+
+    return 0;
+}
+
+const FFCodec ff_farbfeld_encoder = {
+    .p.name         = "farbfeld",
+    CODEC_LONG_NAME("farbfeld uncompressed image"),
+    .p.type         = AVMEDIA_TYPE_VIDEO,
+    .p.id           = AV_CODEC_ID_FARBFELD,
+    .p.capabilities = AV_CODEC_CAP_DR1,
+    FF_CODEC_ENCODE_CB(farbfeld_encode_frame),
+    .p.pix_fmts     = (const enum AVPixelFormat[]){
+        AV_PIX_FMT_RGBA64BE,
+        AV_PIX_FMT_NONE
+    },
+};
diff --git a/libavcodec/version.h b/libavcodec/version.h
index da54f87887..7acb261bb3 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,7 +29,7 @@ 
 
 #include "version_major.h"
 
-#define LIBAVCODEC_VERSION_MINOR   6
+#define LIBAVCODEC_VERSION_MINOR   7
 #define LIBAVCODEC_VERSION_MICRO 100
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
diff --git a/libavformat/img2.c b/libavformat/img2.c
index 9981867f82..77edc7ff9b 100644
--- a/libavformat/img2.c
+++ b/libavformat/img2.c
@@ -95,6 +95,7 @@ 
     TAG(QOI,             qoi      ) \
     TAG(RADIANCE_HDR,    hdr      ) \
     TAG(WBMP,            wbmp     ) \
+    TAG(FARBFELD,        ff       ) \
     TAG(NONE,                     )
 
 #define LENGTH_CHECK(CODECID, STR) \
diff --git a/libavformat/img2enc.c b/libavformat/img2enc.c
index 526a11e5ee..13355a6fad 100644
--- a/libavformat/img2enc.c
+++ b/libavformat/img2enc.c
@@ -276,7 +276,7 @@  const FFOutputFormat ff_image2_muxer = {
     .p.long_name    = NULL_IF_CONFIG_SMALL("image2 sequence"),
     .p.extensions   = "bmp,dpx,exr,jls,jpeg,jpg,jxl,ljpg,pam,pbm,pcx,pfm,pgm,pgmyuv,phm,"
                       "png,ppm,sgi,tga,tif,tiff,jp2,j2c,j2k,xwd,sun,ras,rs,im1,im8,"
-                      "im24,sunras,vbn,xbm,xface,pix,y,avif,qoi,hdr,wbmp",
+                      "im24,sunras,vbn,xbm,xface,pix,y,avif,qoi,hdr,wbmp,ff",
     .priv_data_size = sizeof(VideoMuxData),
     .p.video_codec  = AV_CODEC_ID_MJPEG,
     .write_header   = write_header,