[FFmpeg-devel] [PATCH] avcodec: add farbfeld encoder

[FFmpeg-devel] [PATCH] avcodec: add farbfeld encoder

[Marcus B Spencer]

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 2443d2c6fd..808f2a5d54 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -359,6 +359,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..e48eba680e
--- /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 "libavutil/imgutils.h"
+#include "codec_internal.h"
+#include "bytestream.h"
+#include "avcodec.h"
+#include "encode.h"
+
+#define HEADER_SIZE 16
+
+static int farbfeld_encode_frame(AVCodecContext *ctx, AVPacket *pkt,
+                                 const AVFrame *p, int *got_packet)
+{
+    int pkt_size = HEADER_SIZE + av_image_get_buffer_size(
+        p->format,
+        p->width,
+        p->height,
+        1
+    );
+    uint8_t *buf;
+    int ret;
+
+    if (pkt_size < 0)
+        return pkt_size;
+
+    // 16 is the header size
+    if ((ret = ff_get_encode_buffer(ctx, pkt, pkt_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,
+        pkt_size - HEADER_SIZE,
+        (const uint8_t **)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,
-- 
2.45.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".

Re: [FFmpeg-devel] [PATCH] avcodec: add farbfeld encoder

[Rémi Denis-Courmont]

Le 3 juin 2024 06:27:16 GMT+03:00, Marcus B Spencer <marcus@marcusspencer.xyz> a écrit :
>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/libavcodec/farbfeldenc.c b/libavcodec/farbfeldenc.c
>new file mode 100644
>index 0000000000..e48eba680e
>--- /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 "libavutil/imgutils.h"
>+#include "codec_internal.h"
>+#include "bytestream.h"
>+#include "avcodec.h"
>+#include "encode.h"
>+
>+#define HEADER_SIZE 16
>+
>+static int farbfeld_encode_frame(AVCodecContext *ctx, AVPacket *pkt,
>+                                 const AVFrame *p, int *got_packet)
>+{
>+    int pkt_size = HEADER_SIZE + av_image_get_buffer_size(
>+        p->format,
>+        p->width,
>+        p->height,
>+        1
>+    );
>+    uint8_t *buf;
>+    int ret;
>+
>+    if (pkt_size < 0)
>+        return pkt_size;
>+
>+    // 16 is the header size
>+    if ((ret = ff_get_encode_buffer(ctx, pkt, pkt_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,
>+        pkt_size - HEADER_SIZE,
>+        (const uint8_t **)p->data,

Bogus cast / aliasing violation.

>+        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
>+    },
>+};
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avcodec: add farbfeld encoder

[marcus]

On Monday, June 3rd, 2024 at 1:05 AM, Rémi Denis-Courmont <remi@remlab.net> wrote:

> 
> 
> 
> 
> Le 3 juin 2024 06:27:16 GMT+03:00, Marcus B Spencer marcus@marcusspencer.xyz a écrit :
> 
> > 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/libavcodec/farbfeldenc.c b/libavcodec/farbfeldenc.c
> > new file mode 100644
> > index 0000000000..e48eba680e
> > --- /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 "libavutil/imgutils.h"
> > +#include "codec_internal.h"
> > +#include "bytestream.h"
> > +#include "avcodec.h"
> > +#include "encode.h"
> > +
> > +#define HEADER_SIZE 16
> > +
> > +static int farbfeld_encode_frame(AVCodecContext *ctx, AVPacket *pkt,
> > + const AVFrame *p, int *got_packet)
> > +{
> > + int pkt_size = HEADER_SIZE + av_image_get_buffer_size(
> > + p->format,
> > + p->width,
> > + p->height,
> > + 1
> > + );
> > + uint8_t *buf;
> > + int ret;
> > +
> > + if (pkt_size < 0)
> > + return pkt_size;
> > +
> > + // 16 is the header size
> > + if ((ret = ff_get_encode_buffer(ctx, pkt, pkt_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,
> > + pkt_size - HEADER_SIZE,
> > + (const uint8_t **)p->data,
> 
> 
> Bogus cast / aliasing violation.

I thought qualifiers don't affect aliasing rules in C. Am I wrong?

> 
> > + 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
> > + },
> > +};
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avcodec: add farbfeld encoder

[Rémi Denis-Courmont]

Le maanantaina 3. kesäkuuta 2024, 22.29.13 EEST marcus a écrit :
> > Bogus cast / aliasing violation.
> 
> I thought qualifiers don't affect aliasing rules in C. Am I wrong?

If they didn't, the compiler wouldn't warn about incompatible pointer type 
conversions in the absence of the explict cast.

-- 
Rémi Denis-Courmont
http://www.remlab.net/



_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avcodec: add farbfeld encoder

[Andreas Rheinhardt]

Rémi Denis-Courmont:
> Le maanantaina 3. kesäkuuta 2024, 22.29.13 EEST marcus a écrit :
>>> Bogus cast / aliasing violation.
>>
>> I thought qualifiers don't affect aliasing rules in C. Am I wrong?
> 
> If they didn't, the compiler wouldn't warn about incompatible pointer type 
> conversions in the absence of the explict cast.
> 

Wrong: The conversion uint8_t*const*->const uint8_t * const* is safe
(indeed, C++ performs it automatically); the conversion
uint8_t*const*->const uint8_t ** (which is what the cast here does) is
safe from an aliasing point of view, but not
const-correct/-Wcast-qual-correct (cast to const uint8_t * const* instead).
It is a limitation of C not to be like C++ here.

- Andreas

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avcodec: add farbfeld encoder

[Rémi Denis-Courmont]

Le 4 juin 2024 00:56:21 GMT+03:00, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> a écrit :
>Rémi Denis-Courmont:
>> Le maanantaina 3. kesäkuuta 2024, 22.29.13 EEST marcus a écrit :
>>>> Bogus cast / aliasing violation.
>>>
>>> I thought qualifiers don't affect aliasing rules in C. Am I wrong?
>> 
>> If they didn't, the compiler wouldn't warn about incompatible pointer type 
>> conversions in the absence of the explict cast.
>> 
>
>Wrong: The conversion uint8_t*const*->const uint8_t * const* is safe

Nope. The pointer types have the same representation and in practice neither LLVM IR nor GIMPLE can differentiate them, but they are not compatible according to spec (C11 §6.7.6.1).

You can convert `const uint8_t **` to `const uint8_t *const *`, for sure (C11 §6.3.2.3), but that's not the same conversion
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avcodec: add farbfeld encoder

[Andreas Rheinhardt]

Rémi Denis-Courmont:
> 
> 
> Le 4 juin 2024 00:56:21 GMT+03:00, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> a écrit :
>> Rémi Denis-Courmont:
>>> Le maanantaina 3. kesäkuuta 2024, 22.29.13 EEST marcus a écrit :
>>>>> Bogus cast / aliasing violation.
>>>>
>>>> I thought qualifiers don't affect aliasing rules in C. Am I wrong?
>>>
>>> If they didn't, the compiler wouldn't warn about incompatible pointer type 
>>> conversions in the absence of the explict cast.
>>>
>>
>> Wrong: The conversion uint8_t*const*->const uint8_t * const* is safe
> 
> Nope. The pointer types have the same representation and in practice neither LLVM IR nor GIMPLE can differentiate them, but they are not compatible according to spec (C11 §6.7.6.1).
> 
> You can convert `const uint8_t **` to `const uint8_t *const *`, for sure (C11 §6.3.2.3), but that's not the same conversion

"Similarly, pointers to qualified or unqualified versions of compatible
types shall have the same representation and alignment requirements."
(C11 6.2.5 28) This implies that the cast uint8_t*const*->const uint8_t
* const* obeys the alignment requirement from 6.3.2.3 7.
(It does not imply that the cast is a no-op; these two types may have
different object representations, although I don't think that this has
ever happened for any practical implementation.)

When av_image_copy_to_buffer() dereferences this pointer, it will read
from the array of pointers via an lvalue expression of type const
uint8_t* and this is legal because the effective type is uint8_t* and
reading via a qualified type is fine ("An object shall have its stored
value accessed only by an lvalue expression that has one of the
following types: [...] a qualified version of a type compatible with the
effective type of the object" 6.5 7) Given that const uint8_t* and
uint8_t* have the same size, the same is true when accessing via
pointer+offset.

- Andreas

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avcodec: add farbfeld encoder

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Rémi Denis-Courmont:
>>
>>
>> Le 4 juin 2024 00:56:21 GMT+03:00, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> a écrit :
>>> Rémi Denis-Courmont:
>>>> Le maanantaina 3. kesäkuuta 2024, 22.29.13 EEST marcus a écrit :
>>>>>> Bogus cast / aliasing violation.
>>>>>
>>>>> I thought qualifiers don't affect aliasing rules in C. Am I wrong?
>>>>
>>>> If they didn't, the compiler wouldn't warn about incompatible pointer type 
>>>> conversions in the absence of the explict cast.
>>>>
>>>
>>> Wrong: The conversion uint8_t*const*->const uint8_t * const* is safe
>>
>> Nope. The pointer types have the same representation and in practice neither LLVM IR nor GIMPLE can differentiate them, but they are not compatible according to spec (C11 §6.7.6.1).
>>
>> You can convert `const uint8_t **` to `const uint8_t *const *`, for sure (C11 §6.3.2.3), but that's not the same conversion
> 
> "Similarly, pointers to qualified or unqualified versions of compatible
> types shall have the same representation and alignment requirements."
> (C11 6.2.5 28) This implies that the cast uint8_t*const*->const uint8_t
> * const* obeys the alignment requirement from 6.3.2.3 7.
> (It does not imply that the cast is a no-op; these two types may have
> different object representations, although I don't think that this has
> ever happened for any practical implementation.)
> 
> When av_image_copy_to_buffer() dereferences this pointer, it will read
> from the array of pointers via an lvalue expression of type const
> uint8_t* and this is legal because the effective type is uint8_t* and
> reading via a qualified type is fine ("An object shall have its stored
> value accessed only by an lvalue expression that has one of the
> following types: [...] a qualified version of a type compatible with the
> effective type of the object" 6.5 7) Given that const uint8_t* and
> uint8_t* have the same size, the same is true when accessing via
> pointer+offset.
> 

Wait, the second part is wrong: const uint8_t* and uint8_t* are not
qualified versions of a common type. So there is a effective type
problem, but ABI-wise it is fine by 6.2.5 28. I regard this as a defect
of the spec (just like the fact that the conversion uint8_t**->const
uint8_t*const* is not performed automatically) and not a real issue.

- Andreas

_______________________________________________
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".