[FFmpeg-devel] libavcodec: r12b decoder
Dennis Fleurbaaij
mail at dennisfleurbaaij.com
Tue Jun 8 20:45:35 EEST 2021
Thanks again for the review Andreas. I've addressed the comments:
- moved back to the C90+ style which was in other decoders as well.
- The buffer size check makes sense and I've added that, I'm unsure
about how ffmpeg exactly handles alignment in some cases so I've opted to
stay on the lenient side and only check if it's too small (I see that
pattern more often such as in in r210.c)
- Once you said "shift", I understood what you meant with the "& 0xF0"
comment, it's not about the interaction with the data before it, which I
was thinking you meant, it's just totally not needed and I agree with that.
So that has been cleaned up.
- removed two unused includes
- updated to the latest master
(There are warnings generated during build as Valerii also points out in
his reply, in the previous patch it was md5, this time there are a few
others. These do not stem from this PR for as far as I can see.)
Kind regards,
Dennis Fleurbaaij
On Mon, Jun 7, 2021 at 10:33 PM Andreas Rheinhardt <
andreas.rheinhardt at outlook.com> wrote:
> Dennis Fleurbaaij:
> > Thanks for the review Andreas!
> >
> > I've addressed all your concerns besides the "& in the define", I didn't
> > know that the binary AND is implicit in this situation, any link for
> this?
> > Even if it is, I just find it much more pleasing to see the & there
> > somehow, is there some leniency for personal preference?
> >
>
> The lower nibble doesn't survive the right shift; and given that
> GET_FF() returns an uint8_t, there are no bits higher than 0xFF anyway.
>
> I have no objection to keeping the "&".
>
> >
> > + for (h = 0; h < avctx->height; h++) {
> > + g_dst = (uint16_t *)g_line;
> > + b_dst = (uint16_t *)b_line;
> > + r_dst = (uint16_t *)r_line;
> > +
>
> C90 disallows declarations in the middle of blocks, but it allows them
> at the beginning of *any* block, not only the outermost block of a
> function. You can declare these variables here.
>
> And we also allow for loops with variable declarations, i.e. you may
> declare h (and w later) via "for (int h = 0;". (This is illegal in C90.)
>
>
> Your code currently does not check the size of the input packet: It
> needs to be at least height * width / PIXELS_PER_BLOCK *
> BYTES_PER_BLOCK, but it doesn't check that (and I guess that too big
> packets are also invalid?).
>
> - Andreas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
-------------- next part --------------
From f66434cd65c5590c500e49eb6338aaa3a1c71a80 Mon Sep 17 00:00:00 2001
From: Dennis Fleurbaaij <mail at dennisfleurbaaij.com>
Date: Tue, 8 Jun 2021 18:55:02 +0200
Subject: [PATCH] libavcodec: r12b decoder added
R12B is a format used by BlackMagic DeckLink cards, it is
a big-endian 12bpp RGB format which packs 8 pixels into 36
bytes.
Signed-off-by: Dennis Fleurbaaij <mail at dennisfleurbaaij.com>
---
Changelog | 1 +
libavcodec/Makefile | 1 +
libavcodec/allcodecs.c | 1 +
libavcodec/codec_desc.c | 7 ++
libavcodec/codec_id.h | 1 +
libavcodec/r12bdec.c | 141 ++++++++++++++++++++++++++++++++++++++++
libavcodec/version.h | 4 +-
libavformat/riff.c | 1 +
8 files changed, 155 insertions(+), 2 deletions(-)
create mode 100644 libavcodec/r12bdec.c
diff --git a/Changelog b/Changelog
index b9d5188cf6..d0717072eb 100644
--- a/Changelog
+++ b/Changelog
@@ -7,6 +7,7 @@ version <next>:
- ADPCM IMA Acorn Replay decoder
- Argonaut Games CVG demuxer
- Argonaut Games CVG muxer
+- r12b decoder
version 4.4:
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 4fa8d7ab10..cfef9d57ff 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -575,6 +575,7 @@ OBJS-$(CONFIG_QTRLE_ENCODER) += qtrleenc.o
OBJS-$(CONFIG_R10K_DECODER) += r210dec.o
OBJS-$(CONFIG_R10K_ENCODER) += r210enc.o
OBJS-$(CONFIG_R210_DECODER) += r210dec.o
+OBJS-$(CONFIG_R12B_DECODER) += r12bdec.o
OBJS-$(CONFIG_R210_ENCODER) += r210enc.o
OBJS-$(CONFIG_RA_144_DECODER) += ra144dec.o ra144.o celp_filters.o
OBJS-$(CONFIG_RA_144_ENCODER) += ra144enc.o ra144.o celp_filters.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 623db2a9fa..8db7e730a6 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -266,6 +266,7 @@ extern const AVCodec ff_qtrle_encoder;
extern const AVCodec ff_qtrle_decoder;
extern const AVCodec ff_r10k_encoder;
extern const AVCodec ff_r10k_decoder;
+extern const AVCodec ff_r12b_decoder;
extern const AVCodec ff_r210_encoder;
extern const AVCodec ff_r210_decoder;
extern const AVCodec ff_rasc_decoder;
diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index 35527dcc37..ef58d73576 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -1856,6 +1856,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
.long_name = NULL_IF_CONFIG_SMALL("Digital Pictures SGA Video"),
.props = AV_CODEC_PROP_LOSSY,
},
+ {
+ .id = AV_CODEC_ID_R12B,
+ .type = AVMEDIA_TYPE_VIDEO,
+ .name = "r12b",
+ .long_name = NULL_IF_CONFIG_SMALL("Uncompressed RGB 12-bit 8px in 36B"),
+ .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 83e1dbb4b3..ecfdbc46c0 100644
--- a/libavcodec/codec_id.h
+++ b/libavcodec/codec_id.h
@@ -306,6 +306,7 @@ enum AVCodecID {
AV_CODEC_ID_CRI,
AV_CODEC_ID_SIMBIOSIS_IMX,
AV_CODEC_ID_SGA_VIDEO,
+ AV_CODEC_ID_R12B,
/* various PCM "codecs" */
AV_CODEC_ID_FIRST_AUDIO = 0x10000, ///< A dummy id pointing at the start of audio codecs
diff --git a/libavcodec/r12bdec.c b/libavcodec/r12bdec.c
new file mode 100644
index 0000000000..986f96e687
--- /dev/null
+++ b/libavcodec/r12bdec.c
@@ -0,0 +1,141 @@
+/*
+ * r12b decoder
+ *
+ * Copyright (c) 2021 Dennis Fleurbaaij <mail at dennisfleurbaaij.com>
+ *
+ * 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
+ */
+
+#include "avcodec.h"
+#include "internal.h"
+
+#define WORDS_PER_BLOCK 9
+#define PIXELS_PER_BLOCK 8
+#define BYTES_PER_BLOCK 36
+
+static av_cold int decode_init(AVCodecContext *avctx)
+{
+ avctx->pix_fmt = AV_PIX_FMT_GBRP12LE;
+ avctx->bits_per_raw_sample = 12;
+
+ return 0;
+}
+
+static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
+ AVPacket *avpkt)
+{
+ int ret;
+ uint8_t *g_line, *b_line, *r_line;
+
+ AVFrame *pic = data;
+ const uint8_t* src = avpkt->data;
+ const int blocks_per_line = avctx->width / PIXELS_PER_BLOCK;
+
+ if (avctx->width % PIXELS_PER_BLOCK != 0) {
+ av_log(avctx, AV_LOG_ERROR, "image width not modulo 8\n");
+ return AVERROR_INVALIDDATA;
+ }
+
+ if (avpkt->size < avctx->height * blocks_per_line * BYTES_PER_BLOCK) {
+ av_log(avctx, AV_LOG_ERROR, "packet too small\n");
+ return AVERROR_INVALIDDATA;
+ }
+
+ pic->pict_type = AV_PICTURE_TYPE_I;
+ pic->key_frame = 1;
+
+ if ((ret = ff_get_buffer(avctx, pic, 0)) < 0)
+ return ret;
+
+ g_line = pic->data[0];
+ b_line = pic->data[1];
+ r_line = pic->data[2];
+
+ for (int h = 0; h < avctx->height; h++) {
+ uint16_t *g_dst = (uint16_t *)g_line;
+ uint16_t *b_dst = (uint16_t *)b_line;
+ uint16_t *r_dst = (uint16_t *)r_line;
+
+ for (int w = 0; w < blocks_per_line; w++) {
+
+ // This is an encoding from the table on page 213 of the BlackMagic
+ // Decklink SDK pdf, version 12.0. Few helper defines to directly link
+ // the naming in the doc to the code.
+
+ #define GET_FF(word, byte) (*(src + ((word * 4) + byte)))
+ #define GET_0F(word, byte) (GET_FF(word, byte) & 0x0F)
+ #define GET_F0(word, byte) (GET_FF(word, byte) >> 4)
+ #define PUT(dst, pixel) (*(dst + pixel))
+
+ PUT(b_dst, 0) = GET_FF(0, 0) | GET_0F(1, 3) << 8;
+ PUT(g_dst, 0) = GET_F0(0, 2) | GET_FF(0, 1) << 4;
+ PUT(r_dst, 0) = GET_FF(0, 3) | GET_0F(0, 2) << 8;
+
+ PUT(b_dst, 1) = GET_F0(1, 0) | GET_FF(2, 3) << 4;
+ PUT(g_dst, 1) = GET_FF(1, 1) | GET_0F(1, 0) << 8;
+ PUT(r_dst, 1) = GET_F0(1, 3) | GET_FF(1, 2) << 4;
+
+ PUT(b_dst, 2) = GET_FF(3, 3) | GET_0F(3, 2) << 8;
+ PUT(g_dst, 2) = GET_F0(2, 1) | GET_FF(2, 0) << 4;
+ PUT(r_dst, 2) = GET_FF(2, 2) | GET_0F(2, 1) << 8;
+
+ PUT(b_dst, 3) = GET_F0(4, 3) | GET_FF(4, 2) << 4;
+ PUT(g_dst, 3) = GET_FF(3, 0) | GET_0F(4, 3) << 8;
+ PUT(r_dst, 3) = GET_F0(3, 2) | GET_FF(3, 1) << 4;
+
+ PUT(b_dst, 4) = GET_FF(5, 2) | GET_0F(5, 1) << 8;
+ PUT(g_dst, 4) = GET_F0(4, 0) | GET_FF(5, 3) << 4;
+ PUT(r_dst, 4) = GET_FF(4, 1) | GET_0F(4, 0) << 8;
+
+ PUT(b_dst, 5) = GET_F0(6, 2) | GET_FF(6, 1) << 4;
+ PUT(g_dst, 5) = GET_FF(6, 3) | GET_0F(6, 2) << 8;
+ PUT(r_dst, 5) = GET_F0(5, 1) | GET_FF(5, 0) << 4;
+
+ PUT(b_dst, 6) = GET_FF(7, 1) | GET_0F(7, 0) << 8;
+ PUT(g_dst, 6) = GET_F0(7, 3) | GET_FF(7, 2) << 4;
+ PUT(r_dst, 6) = GET_FF(6, 0) | GET_0F(7, 3) << 8;
+
+ PUT(b_dst, 7) = GET_F0(8, 1) | GET_FF(8, 0) << 4;
+ PUT(g_dst, 7) = GET_FF(8, 2) | GET_0F(8, 1) << 8;
+ PUT(r_dst, 7) = GET_F0(7, 0) | GET_FF(8, 3) << 4;
+
+ src += BYTES_PER_BLOCK;
+ b_dst += PIXELS_PER_BLOCK;
+ g_dst += PIXELS_PER_BLOCK;
+ r_dst += PIXELS_PER_BLOCK;
+ }
+
+ g_line += pic->linesize[0];
+ b_line += pic->linesize[1];
+ r_line += pic->linesize[2];
+ }
+
+ *got_frame = 1;
+
+ return avpkt->size;
+}
+
+const AVCodec ff_r12b_decoder = {
+ .name = "r12b",
+ .long_name = NULL_IF_CONFIG_SMALL("Uncompressed RGB 12-bit 8px in 36B"),
+ .type = AVMEDIA_TYPE_VIDEO,
+ .id = AV_CODEC_ID_R12B,
+ .init = decode_init,
+ .decode = decode_frame,
+ .capabilities = AV_CODEC_CAP_DR1,
+ .caps_internal = FF_CODEC_CAP_INIT_THREADSAFE,
+};
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 5b1e9e77f3..1288cecebe 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -28,8 +28,8 @@
#include "libavutil/version.h"
#define LIBAVCODEC_VERSION_MAJOR 59
-#define LIBAVCODEC_VERSION_MINOR 1
-#define LIBAVCODEC_VERSION_MICRO 101
+#define LIBAVCODEC_VERSION_MINOR 2
+#define LIBAVCODEC_VERSION_MICRO 100
#define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
LIBAVCODEC_VERSION_MINOR, \
diff --git a/libavformat/riff.c b/libavformat/riff.c
index 270ff7c024..6e303f59ff 100644
--- a/libavformat/riff.c
+++ b/libavformat/riff.c
@@ -295,6 +295,7 @@ const AVCodecTag ff_codec_bmp_tags[] = {
{ AV_CODEC_ID_FRWU, MKTAG('F', 'R', 'W', 'U') },
{ AV_CODEC_ID_R10K, MKTAG('R', '1', '0', 'k') },
{ AV_CODEC_ID_R210, MKTAG('r', '2', '1', '0') },
+ { AV_CODEC_ID_R12B, MKTAG('r', '1', '2', 'b') },
{ AV_CODEC_ID_V210, MKTAG('v', '2', '1', '0') },
{ AV_CODEC_ID_V210, MKTAG('C', '2', '1', '0') },
{ AV_CODEC_ID_V308, MKTAG('v', '3', '0', '8') },
--
2.31.1.windows.1
More information about the ffmpeg-devel
mailing list