diff mbox

[FFmpeg-devel,v4,1/2] codec: bitpacked: add decoder

Message ID 20170331153616.7097-1-damien.riegel@savoirfairelinux.com
State Accepted
Headers show

Commit Message

Damien Riegel March 31, 2017, 3:36 p.m. UTC
Add a codec capable of decoding some formats of the RFC4175. For now
it's only capable of handling YCbCr-4:2:2 with 8-bit or 10-bit depth.

For 8-bit it's a simple pass-through, for 10-bit it depacks the stream
in the AV_PIX_FMT_YUV422P10 pixel format.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
---
Changes in v4:
 - use uint64_t for comparison between frame_size and packet_size
 - add a check to make sure width is even
 - take into account linesize to compute buffer addresses
 - use AVERROR_INVALIDDATA instead of custom error codes

Changes in v3:
 - Codec has been renamed bitpacked (instead of vrawdepay)
 - A decoding function is now chosen at codec init based on the pixel
   format
 - Codec marked as experimental

 libavcodec/Makefile     |   1 +
 libavcodec/allcodecs.c  |   1 +
 libavcodec/avcodec.h    |   1 +
 libavcodec/bitpacked.c  | 153 ++++++++++++++++++++++++++++++++++++++++++++++++
 libavcodec/codec_desc.c |   7 +++
 5 files changed, 163 insertions(+)
 create mode 100644 libavcodec/bitpacked.c

Comments

Rostislav Pehlivanov March 31, 2017, 4:51 p.m. UTC | #1
On 31 March 2017 at 16:36, Damien Riegel <damien.riegel@savoirfairelinux.com
> wrote:

> Add a codec capable of decoding some formats of the RFC4175. For now
> it's only capable of handling YCbCr-4:2:2 with 8-bit or 10-bit depth.
>
> For 8-bit it's a simple pass-through, for 10-bit it depacks the stream
> in the AV_PIX_FMT_YUV422P10 pixel format.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> ---
> Changes in v4:
>  - use uint64_t for comparison between frame_size and packet_size
>  - add a check to make sure width is even
>  - take into account linesize to compute buffer addresses
>  - use AVERROR_INVALIDDATA instead of custom error codes
>
> Changes in v3:
>  - Codec has been renamed bitpacked (instead of vrawdepay)
>  - A decoding function is now chosen at codec init based on the pixel
>    format
>  - Codec marked as experimental
>
>
Don't mark it as experimental. The experimental flag is only used for
encoders to avoid picking ones which may generate invalid bitstreams
instead of reference implementation external libraries.
Kieran Kunhya March 31, 2017, 5:11 p.m. UTC | #2
On Fri, 31 Mar 2017 at 17:57 Rostislav Pehlivanov <atomnuker@gmail.com>
wrote:

> On 31 March 2017 at 16:36, Damien Riegel <
> damien.riegel@savoirfairelinux.com
> > wrote:
>
> > Add a codec capable of decoding some formats of the RFC4175. For now
> > it's only capable of handling YCbCr-4:2:2 with 8-bit or 10-bit depth.
> >
> > For 8-bit it's a simple pass-through, for 10-bit it depacks the stream
> > in the AV_PIX_FMT_YUV422P10 pixel format.
> >
> > Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> > ---
> > Changes in v4:
> >  - use uint64_t for comparison between frame_size and packet_size
> >  - add a check to make sure width is even
> >  - take into account linesize to compute buffer addresses
> >  - use AVERROR_INVALIDDATA instead of custom error codes
> >
> > Changes in v3:
> >  - Codec has been renamed bitpacked (instead of vrawdepay)
> >  - A decoding function is now chosen at codec init based on the pixel
> >    format
> >  - Codec marked as experimental
> >
> >
> Don't mark it as experimental. The experimental flag is only used for
> encoders to avoid picking ones which may generate invalid bitstreams
> instead of reference implementation external libraries
>

I asked for it to be marked as experimental, it's not even a real codec, it
doesn't handle any of the timestamp difficulties for example during
wraparound.

Afaik this patch allows for stuff like raw bitpacked, bitpacked in
container X etc.

Kieran
Rostislav Pehlivanov March 31, 2017, 6:23 p.m. UTC | #3
On 31 March 2017 at 18:11, Kieran Kunhya <kierank@obe.tv> wrote:

> On Fri, 31 Mar 2017 at 17:57 Rostislav Pehlivanov <atomnuker@gmail.com>
> wrote:
>
> > On 31 March 2017 at 16:36, Damien Riegel <
> > damien.riegel@savoirfairelinux.com
> > > wrote:
> >
> > > Add a codec capable of decoding some formats of the RFC4175. For now
> > > it's only capable of handling YCbCr-4:2:2 with 8-bit or 10-bit depth.
> > >
> > > For 8-bit it's a simple pass-through, for 10-bit it depacks the stream
> > > in the AV_PIX_FMT_YUV422P10 pixel format.
> > >
> > > Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> > > ---
> > > Changes in v4:
> > >  - use uint64_t for comparison between frame_size and packet_size
> > >  - add a check to make sure width is even
> > >  - take into account linesize to compute buffer addresses
> > >  - use AVERROR_INVALIDDATA instead of custom error codes
> > >
> > > Changes in v3:
> > >  - Codec has been renamed bitpacked (instead of vrawdepay)
> > >  - A decoding function is now chosen at codec init based on the pixel
> > >    format
> > >  - Codec marked as experimental
> > >
> > >
> > Don't mark it as experimental. The experimental flag is only used for
> > encoders to avoid picking ones which may generate invalid bitstreams
> > instead of reference implementation external libraries
> >
>
> I asked for it to be marked as experimental, it's not even a real codec, it
> doesn't handle any of the timestamp difficulties for example during
> wraparound.
>
> Afaik this patch allows for stuff like raw bitpacked, bitpacked in
> container X etc.
>
> Kieran
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


Didn't know it didn't do that, I guess it's okay then. When its fixed it
can be unmarked. We still have some experimental features in the psd
decoder too.
Damien Riegel April 4, 2017, 5:33 p.m. UTC | #4
Hi,

On Fri, Mar 31, 2017 at 07:23:28PM +0100, Rostislav Pehlivanov wrote:
> On 31 March 2017 at 18:11, Kieran Kunhya <kierank@obe.tv> wrote:
> 
> > On Fri, 31 Mar 2017 at 17:57 Rostislav Pehlivanov <atomnuker@gmail.com>
> > wrote:
> >
> > > On 31 March 2017 at 16:36, Damien Riegel <
> > > damien.riegel@savoirfairelinux.com
> > > > wrote:
> > >
> > > > Add a codec capable of decoding some formats of the RFC4175. For now
> > > > it's only capable of handling YCbCr-4:2:2 with 8-bit or 10-bit depth.
> > > >
> > > > For 8-bit it's a simple pass-through, for 10-bit it depacks the stream
> > > > in the AV_PIX_FMT_YUV422P10 pixel format.
> > > >
> > > > Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> > > > ---
> > > > Changes in v4:
> > > >  - use uint64_t for comparison between frame_size and packet_size
> > > >  - add a check to make sure width is even
> > > >  - take into account linesize to compute buffer addresses
> > > >  - use AVERROR_INVALIDDATA instead of custom error codes
> > > >
> > > > Changes in v3:
> > > >  - Codec has been renamed bitpacked (instead of vrawdepay)
> > > >  - A decoding function is now chosen at codec init based on the pixel
> > > >    format
> > > >  - Codec marked as experimental
> > > >
> > > >
> > > Don't mark it as experimental. The experimental flag is only used for
> > > encoders to avoid picking ones which may generate invalid bitstreams
> > > instead of reference implementation external libraries
> > >
> >
> > I asked for it to be marked as experimental, it's not even a real codec, it
> > doesn't handle any of the timestamp difficulties for example during
> > wraparound.
> >
> > Afaik this patch allows for stuff like raw bitpacked, bitpacked in
> > container X etc.
> >
> > Kieran
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> 
> 
> Didn't know it didn't do that, I guess it's okay then. When its fixed it
> can be unmarked. We still have some experimental features in the psd
> decoder too.

Indeed I added the flag following Kieran's suggestion. Is there anything
else I could do to improve this patchset?

Thanks,
Rostislav Pehlivanov April 5, 2017, 4:10 p.m. UTC | #5
On 4 April 2017 at 18:33, Damien Riegel <damien.riegel@savoirfairelinux.com>
wrote:

> Hi,
>
> On Fri, Mar 31, 2017 at 07:23:28PM +0100, Rostislav Pehlivanov wrote:
> > On 31 March 2017 at 18:11, Kieran Kunhya <kierank@obe.tv> wrote:
> >
> > > On Fri, 31 Mar 2017 at 17:57 Rostislav Pehlivanov <atomnuker@gmail.com
> >
> > > wrote:
> > >
> > > > On 31 March 2017 at 16:36, Damien Riegel <
> > > > damien.riegel@savoirfairelinux.com
> > > > > wrote:
> > > >
> > > > > Add a codec capable of decoding some formats of the RFC4175. For
> now
> > > > > it's only capable of handling YCbCr-4:2:2 with 8-bit or 10-bit
> depth.
> > > > >
> > > > > For 8-bit it's a simple pass-through, for 10-bit it depacks the
> stream
> > > > > in the AV_PIX_FMT_YUV422P10 pixel format.
> > > > >
> > > > > Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> > > > > ---
> > > > > Changes in v4:
> > > > >  - use uint64_t for comparison between frame_size and packet_size
> > > > >  - add a check to make sure width is even
> > > > >  - take into account linesize to compute buffer addresses
> > > > >  - use AVERROR_INVALIDDATA instead of custom error codes
> > > > >
> > > > > Changes in v3:
> > > > >  - Codec has been renamed bitpacked (instead of vrawdepay)
> > > > >  - A decoding function is now chosen at codec init based on the
> pixel
> > > > >    format
> > > > >  - Codec marked as experimental
> > > > >
> > > > >
> > > > Don't mark it as experimental. The experimental flag is only used for
> > > > encoders to avoid picking ones which may generate invalid bitstreams
> > > > instead of reference implementation external libraries
> > > >
> > >
> > > I asked for it to be marked as experimental, it's not even a real
> codec, it
> > > doesn't handle any of the timestamp difficulties for example during
> > > wraparound.
> > >
> > > Afaik this patch allows for stuff like raw bitpacked, bitpacked in
> > > container X etc.
> > >
> > > Kieran
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel@ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> >
> >
> > Didn't know it didn't do that, I guess it's okay then. When its fixed it
> > can be unmarked. We still have some experimental features in the psd
> > decoder too.
>
> Indeed I added the flag following Kieran's suggestion. Is there anything
> else I could do to improve this patchset?
>
> Thanks,
> --
> Damien
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Pushed, thanks!
diff mbox

Patch

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 876a69e013..7a4eeaeac1 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -214,6 +214,7 @@  OBJS-$(CONFIG_BINK_DECODER)            += bink.o binkdsp.o
 OBJS-$(CONFIG_BINKAUDIO_DCT_DECODER)   += binkaudio.o
 OBJS-$(CONFIG_BINKAUDIO_RDFT_DECODER)  += binkaudio.o
 OBJS-$(CONFIG_BINTEXT_DECODER)         += bintext.o cga_data.o
+OBJS-$(CONFIG_BITPACKED_DECODER)       += bitpacked.o
 OBJS-$(CONFIG_BMP_DECODER)             += bmp.o msrledec.o
 OBJS-$(CONFIG_BMP_ENCODER)             += bmpenc.o
 OBJS-$(CONFIG_BMV_AUDIO_DECODER)       += bmvaudio.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index b7d03ad601..a746960add 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -362,6 +362,7 @@  static void register_all(void)
     REGISTER_DECODER(VP8,               vp8);
     REGISTER_DECODER(VP9,               vp9);
     REGISTER_DECODER(VQA,               vqa);
+    REGISTER_DECODER(BITPACKED,         bitpacked);
     REGISTER_DECODER(WEBP,              webp);
     REGISTER_ENCODER(WRAPPED_AVFRAME,   wrapped_avframe);
     REGISTER_ENCDEC (WMV1,              wmv1);
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 60f7acefbd..7a26bf00d0 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -442,6 +442,7 @@  enum AVCodecID {
     AV_CODEC_ID_CLEARVIDEO,
     AV_CODEC_ID_XPM,
     AV_CODEC_ID_AV1,
+    AV_CODEC_ID_BITPACKED,
 
     /* various PCM "codecs" */
     AV_CODEC_ID_FIRST_AUDIO = 0x10000,     ///< A dummy id pointing at the start of audio codecs
diff --git a/libavcodec/bitpacked.c b/libavcodec/bitpacked.c
new file mode 100644
index 0000000000..949d20c961
--- /dev/null
+++ b/libavcodec/bitpacked.c
@@ -0,0 +1,153 @@ 
+/*
+ * Unpack bit-packed streams to formats supported by FFmpeg
+ * Copyright (c) 2017 Savoir-faire Linux, 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
+ */
+
+/* Development sponsored by CBC/Radio-Canada */
+
+/**
+ * @file
+ * Bitpacked
+ */
+
+#include "avcodec.h"
+#include "internal.h"
+#include "get_bits.h"
+#include "libavutil/imgutils.h"
+
+struct BitpackedContext {
+    int (*decode)(AVCodecContext *avctx, AVFrame *frame,
+                  AVPacket *pkt);
+};
+
+/* For this format, it's a simple passthrough */
+static int bitpacked_decode_uyvy422(AVCodecContext *avctx, AVFrame *frame,
+                                    AVPacket *avpkt)
+{
+    int ret;
+
+    /* there is no need to copy as the data already match
+     * a known pixel format */
+    frame->buf[0] = av_buffer_ref(avpkt->buf);
+    ret = av_image_fill_arrays(frame->data, frame->linesize, avpkt->data,
+                               avctx->pix_fmt, avctx->width, avctx->height, 1);
+    if (ret < 0) {
+        av_buffer_unref(&frame->buf[0]);
+        return ret;
+    }
+
+    return 0;
+}
+
+static int bitpacked_decode_yuv422p10(AVCodecContext *avctx, AVFrame *frame,
+                                      AVPacket *avpkt)
+{
+    uint64_t frame_size = (uint64_t)avctx->width * (uint64_t)avctx->height * 20;
+    uint64_t packet_size = avpkt->size * 8;
+    GetBitContext bc;
+    uint16_t *y, *u, *v;
+    int ret, i;
+
+    ret = ff_get_buffer(avctx, frame, 0);
+    if (ret < 0)
+        return ret;
+
+    y = (uint16_t*)frame->data[0];
+    u = (uint16_t*)frame->data[1];
+    v = (uint16_t*)frame->data[2];
+
+    if (frame_size > packet_size)
+        return AVERROR_INVALIDDATA;
+
+    if (avctx->width % 2)
+        return AVERROR_PATCHWELCOME;
+
+    ret = init_get_bits(&bc, avpkt->data, avctx->width * avctx->height * 20);
+    if (ret)
+        return ret;
+
+    for (i = 0; i < avctx->height; i++) {
+        y = (uint16_t*)(frame->data[0] + i * frame->linesize[0]);
+        u = (uint16_t*)(frame->data[1] + i * frame->linesize[1]);
+        v = (uint16_t*)(frame->data[2] + i * frame->linesize[2]);
+
+        for (int j = 0; j < avctx->width; j += 2) {
+            *u++ = get_bits(&bc, 10);
+            *y++ = get_bits(&bc, 10);
+            *v++ = get_bits(&bc, 10);
+            *y++ = get_bits(&bc, 10);
+        }
+    }
+
+    return 0;
+}
+
+static av_cold int bitpacked_init_decoder(AVCodecContext *avctx)
+{
+    struct BitpackedContext *bc = avctx->priv_data;
+
+    if (!avctx->codec_tag || !avctx->width || !avctx->height)
+        return AVERROR_INVALIDDATA;
+
+    if (avctx->codec_tag == MKTAG('U', 'Y', 'V', 'Y')) {
+        if (avctx->bits_per_coded_sample == 16 &&
+            avctx->pix_fmt == AV_PIX_FMT_UYVY422)
+            bc->decode = bitpacked_decode_uyvy422;
+        else if (avctx->bits_per_coded_sample == 20 &&
+                 avctx->pix_fmt == AV_PIX_FMT_YUV422P10)
+            bc->decode = bitpacked_decode_yuv422p10;
+        else
+            return AVERROR_INVALIDDATA;
+    } else {
+        return AVERROR_INVALIDDATA;
+    }
+
+    return 0;
+}
+
+static int bitpacked_decode(AVCodecContext *avctx, void *data, int *got_frame,
+                            AVPacket *avpkt)
+{
+    struct BitpackedContext *bc = avctx->priv_data;
+    int buf_size = avpkt->size;
+    AVFrame *frame = data;
+    int res;
+
+    frame->pict_type = AV_PICTURE_TYPE_I;
+    frame->key_frame = 1;
+
+    res = bc->decode(avctx, frame, avpkt);
+    if (res)
+        return res;
+
+    *got_frame = 1;
+    return buf_size;
+
+}
+
+AVCodec ff_bitpacked_decoder = {
+    .name   = "bitpacked",
+    .long_name = NULL_IF_CONFIG_SMALL("Bitpacked"),
+    .type = AVMEDIA_TYPE_VIDEO,
+    .id = AV_CODEC_ID_BITPACKED,
+    .priv_data_size        = sizeof(struct BitpackedContext),
+    .init = bitpacked_init_decoder,
+    .decode = bitpacked_decode,
+    .capabilities = AV_CODEC_CAP_EXPERIMENTAL,
+};
diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index 9711019e9d..4609e2c00e 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -1381,6 +1381,13 @@  static const AVCodecDescriptor codec_descriptors[] = {
         .long_name = NULL_IF_CONFIG_SMALL("Alliance for Open Media AV1"),
         .props     = AV_CODEC_PROP_LOSSY,
     },
+    {
+        .id        = AV_CODEC_ID_BITPACKED,
+        .type      = AVMEDIA_TYPE_VIDEO,
+        .name      = "bitpacked",
+        .long_name = NULL_IF_CONFIG_SMALL("Bitpacked"),
+        .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
+    },
 
     /* image codecs */
     {