[FFmpeg-devel] [PATCH v3 1/2] codec: bitpacked: add decoder

[Damien Riegel]

On Tue, Feb 28, 2017 at 11:10:23PM +0100, Michael Niedermayer wrote:
> On Tue, Feb 28, 2017 at 01:39:07PM -0500, Damien Riegel 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 at savoirfairelinux.com>
> > ---
> > 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  | 148 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  libavcodec/codec_desc.c |   7 +++
> >  5 files changed, 158 insertions(+)
> >  create mode 100644 libavcodec/bitpacked.c
> > 
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index 1bea44ad91..dc5e684f10 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 eee322b2b8..86a8fd56c5 100644
> > --- a/libavcodec/allcodecs.c
> > +++ b/libavcodec/allcodecs.c
> > @@ -366,6 +366,7 @@ void avcodec_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 925a8c7277..a0d07c64bc 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -416,6 +416,7 @@ enum AVCodecID {
> >      AV_CODEC_ID_SPEEDHQ,
> >      AV_CODEC_ID_FMVC,
> >      AV_CODEC_ID_SCPR,
> > +    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..1d935b71a1
> > --- /dev/null
> > +++ b/libavcodec/bitpacked.c
> > @@ -0,0 +1,148 @@
> > +/*
> > + * 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 <libavutil/imgutils.h>
> 
> wrong directory
> 
> 
> > +#include "avcodec.h"
> > +#include "internal.h"
> > +#include "get_bits.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);
> 
> linesize and the initial data pointers have some alignment requirements
> 
> they may need to be checked before pass through

I addressed others comments but I'm not sure I understand this one. What
are these alignment requirements and what needs to be checked?

> > [...]

> > +
> > +    ret = init_get_bits(&bc, avpkt->data, avctx->width * avctx->height * 20);
> > +    if (ret)
> > +        return ret;
> > +
> 
> > +    for (i = 0; i < (avctx->width * avctx->height) / 2; i++) {
> > +        *u++ = get_bits(&bc, 10);
> > +        *y++ = get_bits(&bc, 10);
> > +        *v++ = get_bits(&bc, 10);
> > +        *y++ = get_bits(&bc, 10);
> 
> custom code to read this would likely be faster tha the generic
> bit reader

Probably, but I'd rather have a very straightforward first
implementation. Optimization could come a bit later (like hardware
accelerated unpacking) if that's okay with you.

-- 
Damien