[FFmpeg-devel,v3] libavcodec: implementation of DNxUncompressed decoder

As suggested by Zhao Zhili, here is a squash rebased 3rd attempt.

I'm still unsure about my solution of adding the codec_id entries and
increasing the lead-position value, as criticized by Andreas Rheinhard,
but I don't see any other way to make it better.

martin

---
 Changelog                 |   1 +
 doc/general_contents.texi |   1 +
 libavcodec/Makefile       |   1 +
 libavcodec/allcodecs.c    |   1 +
 libavcodec/codec_desc.c   |   7 +
 libavcodec/codec_id.h     |   1 +
 libavcodec/dnxucdec.c     | 495 ++++++++++++++++++++++++++++++++++++++
 libavcodec/parsers.c      |   1 +
 libavcodec/version.c      |   2 +-
 libavcodec/version.h      |   2 +-
 libavformat/mxf.c         |   1 +
 libavformat/mxfdec.c      |  21 ++
 12 files changed, 532 insertions(+), 2 deletions(-)
 create mode 100644 libavcodec/dnxucdec.c

Quoting Martin Schitter (2024-09-08 20:41:38)
> As suggested by Zhao Zhili, here is a squash rebased 3rd attempt.
> 
> I'm still unsure about my solution of adding the codec_id entries and
> increasing the lead-position value, as criticized by Andreas Rheinhard,
> but I don't see any other way to make it better.

I think you're misunderstanding the meaning of AV_CODEC_ID_LEAD - it's
not in any way special, it's just the ID of a codec called "LEAD MCMP".
You should add your new codec ID immediately after it.

Quoting Martin Schitter (2024-09-08 20:41:38)
> diff --git a/libavcodec/dnxucdec.c b/libavcodec/dnxucdec.c
> new file mode 100644
> index 0000000..455c374
> --- /dev/null
> +++ b/libavcodec/dnxucdec.c
> @@ -0,0 +1,495 @@
> +/*
> + * Avid DNxUncomressed / SMPTE RDD 50 demuxer

This says it's a demuxer, while it's implemented as a decoder.

I'm also wondering if this shouldn't be handled as demuxer exporting
raw video, at least in some of cases if not all.

> + * Copyright (c) 2024 Martin Schitter
> + *
> + * 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
> + */
> +
> +/*
> + This partial implementation of a decoder for DNxUncompressed video data
> + is based on reverse engineering of output generated by DaVinci Resolve 19
> + because the SMPTE RDD 50 specification is unfortunately not freely accessible.
> +
> + It's therefor limited by the present export capabilities of Resolve (YUV444
> + variants, YUV422 16bit, and alpha support is missing). But also some ffmpeg
> + shortcomings are affecting the range of available formats (no YUV half and
> + float pixel formats and filters are provided by ffmpeg until now and RGB half
> + content always requires an alpha plane).
> +
> + A wide range of DNxUncompressed formats are nevertheless already supported:
> +
> +  - YUV 4:2:2 8-/10-/12-bit
> +  - RGB 8-/10-/12-bit/half/float
> +
> +*/
> +
> +#include "avcodec.h"
> +#include "codec_internal.h"
> +#include "decode.h"
> +#include "libavutil/imgutils.h"
> +#include "thread.h"
> +
> +typedef struct DNxUcParseContext {
> +    uint32_t fourcc_tag;
> +    uint32_t width;
> +    uint32_t height;
> +    uint32_t nr_bytes;
> +} DNxUcParseContext;

Parser should be in its own file, as it can be enabled or disabled
independently from the encoder.
> +
> +/*
> +DNxUncompressed frame data comes wrapped in simple metadata
> +and fourcc markers:
> +
> +[0-4]   number of raw data (37 bytes header + frame data)
> +[4-7]   fourcc 'pack'
> +[8-11]  unknown value (allways: 0x15)
> +[12-15] fourcc 'sinf'
> +[16-19] frame width / line packing size
> +[20-23] frame hight / nr of lines
> +[24-27] fourcc pixel format indicator
> +[28]    unknown value (alpha?)
> +[29-32] nr of bytes in frame data + 8
> +[33-36] fourcc 'sdat'
> +[37-..] frame data
> +*/
> +
> +static int dnxuc_parse(AVCodecParserContext *s,
> +                    AVCodecContext *avctx,
> +                    const uint8_t **poutbuf, int *poutbuf_size,
> +                    const uint8_t *buf, int buf_size){

Opening brace of a function body should be on its own line.
> +
> +    char fourcc_buf[5];
> +    const int HEADER_SIZE = 37;
> +
> +    DNxUcParseContext *pc;
> +    pc = (DNxUcParseContext *) s->priv_data;
> +
> +    if (!buf_size) {
> +        return 0;
> +    } else if ( buf_size < 37 /* check metadata structure expectations */
> +        || MKTAG('p','a','c','k') != *(uint32_t*) (buf+4)
> +        || MKTAG('s','i','n','f') != *(uint32_t*) (buf+12)
> +        || MKTAG('s','d','a','t') != *(uint32_t*) (buf+33)){
> +            av_log(0, AV_LOG_ERROR, "can't read DNxUncompressed metadata.\n");

av_log() should always get a proper logging context, avctx in this case

> +            *poutbuf_size = 0;
> +            return buf_size;
> +    }
> +
> +    pc->fourcc_tag = *(uint32_t*)(buf+24);
> +    pc->width =  *(uint32_t*)(buf+16);
> +    pc->height =  *(uint32_t*)(buf+20);
> +    pc->nr_bytes = *(uint32_t*)(buf+29) - 8;

Use AV_RL32()
> +
> +    if (!avctx->codec_tag) {
> +        av_fourcc_make_string(fourcc_buf, pc->fourcc_tag);
> +        av_log(0, AV_LOG_INFO, "dnxuc_parser: '%s' %dx%d %dbpp %d\n",
> +            fourcc_buf,
> +            pc->width, pc->height,
> +            (pc->nr_bytes*8)/(pc->width*pc->height),
> +            pc->nr_bytes);
> +        avctx->codec_tag = pc->fourcc_tag;
> +    }
> +
> +    if (pc->nr_bytes != buf_size - HEADER_SIZE){
> +        av_log(avctx, AV_LOG_ERROR, "Insufficient size of data.\n");
> +        *poutbuf_size = 0;
> +        return buf_size;
> +    }
> +
> +    *poutbuf = buf + HEADER_SIZE;
> +    *poutbuf_size = pc->nr_bytes;
> +
> +    return buf_size;
> +}
> +
> +static av_cold int dnxuc_decode_init(AVCodecContext *avctx){
> +    return 0;
> +}
> +
> +
> +static int pass_though(AVCodecContext *avctx, AVFrame *frame, const AVPacket *avpkt){
> +
> +    /* there is no need to copy as the data already match
> +     * a known pixel format */
> +
> +    frame->buf[0] = av_buffer_ref(avpkt->buf);
> +
> +    if (!frame->buf[0]) {
> +        return AVERROR(ENOMEM);
> +    }
> +
> +    return av_image_fill_arrays(frame->data, frame->linesize, avpkt->data,
> +                               avctx->pix_fmt, avctx->width, avctx->height, 1);
> +}
> +
> +static int float2planes(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt){
> +
> +    int ret, x, y, lw;
> +    const size_t sof = 4;
> +
> +    ret = ff_thread_get_buffer(avctx, frame, 0);
> +    if (ret < 0)
> +        return ret;
> +
> +    lw = frame->width;
> +
> +    for(y = 0; y < frame->height; y++){
> +        for(x = 0; x < frame->width; x++){
> +            memcpy(&frame->data[2][sof*(lw*y + x)], &pkt->data[sof* 3*(lw*y + x)], sof);
> +            memcpy(&frame->data[0][sof*(lw*y + x)], &pkt->data[sof*(3*(lw*y + x) + 1)], sof);
> +            memcpy(&frame->data[1][sof*(lw*y + x)], &pkt->data[sof*(3*(lw*y + x) + 2)], sof);
> +        }
> +    }
> +    return pkt->size;
> +}
> +
> +static int half_add_alpha(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt){
> +
> +    /* ffmpeg doesn't provide RGB half bit depth without alpha channel right now
> +     * we simply add an opaque alpha layer as workaround */
> +
> +    int ret, x, y, lw;
> +    const size_t soh = 2;
> +    const uint16_t opaque = 0x3c00;
> +
> +    ret = ff_thread_get_buffer(avctx, frame, 0);
> +    if (ret < 0)
> +        return ret;

This call should not be duplicated in every handler.

> +
> +    lw = frame->width;
> +
> +    for(y = 0; y < frame->height; y++){
> +        for(x = 0; x < frame->width; x++){
> +            memcpy(&frame->data[0][soh*4*(lw*y + x)], &pkt->data[soh*3*(lw*y + x)], soh*3);
> +            memcpy(&frame->data[0][soh*(4*(lw*y + x) + 3)], &opaque, soh);
> +        }
> +    }
> +    return pkt->size;
> +}
> +
> +/* DNxUncompressed utilizes a very dense bitpack representation of 10bit and 12bit pixel data.
> +
> +Lines of Image data, which look like in their ordinary 8bit counterpart, contain the most
> +significant upper bits of the pixel data. These sections alternate with shorter segments in
> +which the complementary least significant bits of information get packed in a gapless sequence.
> +
> ++----------------------+ +----------------------+ +------------------------+ +----------~
> +|  8 m.s.bits of R[1]  | |  8 m.s.bits of G[1]  | |  8 m.s.bits of B[1]    | | msb R[2]    ... one line
> ++----------------------+ +----------------------+ +------------------------+ +----------~
> +
> ++---------------------------------------------------------------+  +-----------~
> +| +------------+ +------------+ +------------+ +--------------+ |  | +--------~
> +| | 2 lsb R[1] | | 2 lsb G[1] | | 2 lsb B[1] | | 2 lsb R[2]   | |  | | G[2]lsb    ... LSB bits for line
> +| +------------+ +------------+ +------------+ +--------------+ |  | +--------~
> ++---------------------------- one byte ------------------------ +  +-----------~
> +
> +next line of MSB bytes...   */
> +
> +static int unpack_rg10(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt){
> +
> +    int ret, x, y, lw, msp, pack, lsp, p_off;
> +    uint16_t r,g,b;
> +
> +    if (avctx->width % 4){
> +        av_log(0, AV_LOG_ERROR,
> +        "Image width has to be dividable by 4 for 10bit RGB DNxUncompressed!\n");
> +        return AVERROR_EXIT;
> +    }

These checks can be done once during init.
> +
> +    ret = ff_thread_get_buffer(avctx, frame, 0);
> +    if (ret < 0)
> +        return ret;
> +
> +    lw = frame->width;
> +
> +    for(y = 0; y < frame->height; y++){
> +        for(x = 0; x < frame->width; x++){
> +            msp = pkt->data[y*3*(lw + lw/4) + 3*x];

Does anything guarantee the packet is large enough?

On Sun, 8 Sep 2024, Martin Schitter wrote:

> As suggested by Zhao Zhili, here is a squash rebased 3rd attempt.
>
> I'm still unsure about my solution of adding the codec_id entries and
> increasing the lead-position value, as criticized by Andreas Rheinhard,
> but I don't see any other way to make it better.
>
> martin
>
> ---
> Changelog                 |   1 +
> doc/general_contents.texi |   1 +
> libavcodec/Makefile       |   1 +
> libavcodec/allcodecs.c    |   1 +
> libavcodec/codec_desc.c   |   7 +
> libavcodec/codec_id.h     |   1 +
> libavcodec/dnxucdec.c     | 495 ++++++++++++++++++++++++++++++++++++++
> libavcodec/parsers.c      |   1 +
> libavcodec/version.c      |   2 +-
> libavcodec/version.h      |   2 +-
> libavformat/mxf.c         |   1 +
> libavformat/mxfdec.c      |  21 ++
> 12 files changed, 532 insertions(+), 2 deletions(-)
> create mode 100644 libavcodec/dnxucdec.c

You should split this patch to at least 3 parts for easier review. Adding 
the codec ID and description, adding MXF demux support, and adding the 
decoder/parser.

There is a note saying that the specs are not freely accessible, that is 
not the case as far as I see:

https://pub.smpte.org/doc/rdd50/20190730-pub/

So maybe you want to check your work based on the specs?

Thanks,
Marton

>
> diff --git a/Changelog b/Changelog
> index 583de26..fbda69e 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -19,6 +19,7 @@ version <next>:
> - Cropping metadata parsing and writing in Matroska and MP4/MOV de/muxers
> - Intel QSV-accelerated VVC decoding
> - MediaCodec AAC/AMR-NB/AMR-WB/MP3 decoding
> +- DNxUncompressed (SMPTE RDD 50) decoder
>
>
> version 7.0:
> diff --git a/doc/general_contents.texi b/doc/general_contents.texi
> index e7cf4f8..1593124 100644
> --- a/doc/general_contents.texi
> +++ b/doc/general_contents.texi
> @@ -619,6 +619,7 @@ library:
> @item raw DFPWM                 @tab X @tab X
> @item raw Dirac                 @tab X @tab X
> @item raw DNxHD                 @tab X @tab X
> +@itme raw DNxUncompressed       @tab   @tab X
> @item raw DTS                   @tab X @tab X
> @item raw DTS-HD                @tab   @tab X
> @item raw E-AC-3                @tab X @tab X
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 75ae377..cfa8fba 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -325,6 +325,7 @@ OBJS-$(CONFIG_DFPWM_DECODER)           += dfpwmdec.o
> OBJS-$(CONFIG_DFPWM_ENCODER)           += dfpwmenc.o
> OBJS-$(CONFIG_DNXHD_DECODER)           += dnxhddec.o dnxhddata.o
> OBJS-$(CONFIG_DNXHD_ENCODER)           += dnxhdenc.o dnxhddata.o
> +OBJS-$(CONFIG_DNXUC_DECODER)           += dnxucdec.o
> OBJS-$(CONFIG_DOLBY_E_DECODER)         += dolby_e.o dolby_e_parse.o kbdwin.o
> OBJS-$(CONFIG_DPX_DECODER)             += dpx.o
> OBJS-$(CONFIG_DPX_ENCODER)             += dpxenc.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 563afde..ea8f2a4 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -93,6 +93,7 @@ extern const FFCodec ff_dfa_decoder;
> extern const FFCodec ff_dirac_decoder;
> extern const FFCodec ff_dnxhd_encoder;
> extern const FFCodec ff_dnxhd_decoder;
> +extern const FFCodec ff_dnxuc_decoder;
> extern const FFCodec ff_dpx_encoder;
> extern const FFCodec ff_dpx_decoder;
> extern const FFCodec ff_dsicinvideo_decoder;
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index a28ef68..2b83ea2 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -1952,6 +1952,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
>         .long_name = NULL_IF_CONFIG_SMALL("vMix Video"),
>         .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
>     },
> +    {
> +        .id        = AV_CODEC_ID_DNXUC,
> +        .type      = AVMEDIA_TYPE_VIDEO,
> +        .name      = "dnxuc",
> +        .long_name = NULL_IF_CONFIG_SMALL("DNxUncompressed / SMPTE RDD 50"),
> +        .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
> +    },
>     {
>         .id        = AV_CODEC_ID_LEAD,
>         .type      = AVMEDIA_TYPE_VIDEO,
> diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h
> index 0ab1e34..27b229b 100644
> --- a/libavcodec/codec_id.h
> +++ b/libavcodec/codec_id.h
> @@ -321,6 +321,7 @@ enum AVCodecID {
>     AV_CODEC_ID_EVC,
>     AV_CODEC_ID_RTV1,
>     AV_CODEC_ID_VMIX,
> +    AV_CODEC_ID_DNXUC,
>     AV_CODEC_ID_LEAD,
>
>     /* various PCM "codecs" */
> diff --git a/libavcodec/dnxucdec.c b/libavcodec/dnxucdec.c
> new file mode 100644
> index 0000000..455c374
> --- /dev/null
> +++ b/libavcodec/dnxucdec.c
> @@ -0,0 +1,495 @@
> +/*
> + * Avid DNxUncomressed / SMPTE RDD 50 demuxer
> + * Copyright (c) 2024 Martin Schitter
> + *
> + * 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
> + */
> +
> +/*
> + This partial implementation of a decoder for DNxUncompressed video data
> + is based on reverse engineering of output generated by DaVinci Resolve 19
> + because the SMPTE RDD 50 specification is unfortunately not freely accessible.
> +
> + It's therefor limited by the present export capabilities of Resolve (YUV444
> + variants, YUV422 16bit, and alpha support is missing). But also some ffmpeg
> + shortcomings are affecting the range of available formats (no YUV half and
> + float pixel formats and filters are provided by ffmpeg until now and RGB half
> + content always requires an alpha plane).
> +
> + A wide range of DNxUncompressed formats are nevertheless already supported:
> +
> +  - YUV 4:2:2 8-/10-/12-bit
> +  - RGB 8-/10-/12-bit/half/float
> +
> +*/
> +
> +#include "avcodec.h"
> +#include "codec_internal.h"
> +#include "decode.h"
> +#include "libavutil/imgutils.h"
> +#include "thread.h"
> +
> +typedef struct DNxUcParseContext {
> +    uint32_t fourcc_tag;
> +    uint32_t width;
> +    uint32_t height;
> +    uint32_t nr_bytes;
> +} DNxUcParseContext;
> +
> +/*
> +DNxUncompressed frame data comes wrapped in simple metadata
> +and fourcc markers:
> +
> +[0-4]   number of raw data (37 bytes header + frame data)
> +[4-7]   fourcc 'pack'
> +[8-11]  unknown value (allways: 0x15)
> +[12-15] fourcc 'sinf'
> +[16-19] frame width / line packing size
> +[20-23] frame hight / nr of lines
> +[24-27] fourcc pixel format indicator
> +[28]    unknown value (alpha?)
> +[29-32] nr of bytes in frame data + 8
> +[33-36] fourcc 'sdat'
> +[37-..] frame data
> +*/
> +
> +static int dnxuc_parse(AVCodecParserContext *s,
> +                    AVCodecContext *avctx,
> +                    const uint8_t **poutbuf, int *poutbuf_size,
> +                    const uint8_t *buf, int buf_size){
> +
> +    char fourcc_buf[5];
> +    const int HEADER_SIZE = 37;
> +
> +    DNxUcParseContext *pc;
> +    pc = (DNxUcParseContext *) s->priv_data;
> +
> +    if (!buf_size) {
> +        return 0;
> +    } else if ( buf_size < 37 /* check metadata structure expectations */
> +        || MKTAG('p','a','c','k') != *(uint32_t*) (buf+4)
> +        || MKTAG('s','i','n','f') != *(uint32_t*) (buf+12)
> +        || MKTAG('s','d','a','t') != *(uint32_t*) (buf+33)){
> +            av_log(0, AV_LOG_ERROR, "can't read DNxUncompressed metadata.\n");
> +            *poutbuf_size = 0;
> +            return buf_size;
> +    }
> +
> +    pc->fourcc_tag = *(uint32_t*)(buf+24);
> +    pc->width =  *(uint32_t*)(buf+16);
> +    pc->height =  *(uint32_t*)(buf+20);
> +    pc->nr_bytes = *(uint32_t*)(buf+29) - 8;
> +
> +    if (!avctx->codec_tag) {
> +        av_fourcc_make_string(fourcc_buf, pc->fourcc_tag);
> +        av_log(0, AV_LOG_INFO, "dnxuc_parser: '%s' %dx%d %dbpp %d\n",
> +            fourcc_buf,
> +            pc->width, pc->height,
> +            (pc->nr_bytes*8)/(pc->width*pc->height),
> +            pc->nr_bytes);
> +        avctx->codec_tag = pc->fourcc_tag;
> +    }
> +
> +    if (pc->nr_bytes != buf_size - HEADER_SIZE){
> +        av_log(avctx, AV_LOG_ERROR, "Insufficient size of data.\n");
> +        *poutbuf_size = 0;
> +        return buf_size;
> +    }
> +
> +    *poutbuf = buf + HEADER_SIZE;
> +    *poutbuf_size = pc->nr_bytes;
> +
> +    return buf_size;
> +}
> +
> +static av_cold int dnxuc_decode_init(AVCodecContext *avctx){
> +    return 0;
> +}
> +
> +
> +static int pass_though(AVCodecContext *avctx, AVFrame *frame, const AVPacket *avpkt){
> +
> +    /* there is no need to copy as the data already match
> +     * a known pixel format */
> +
> +    frame->buf[0] = av_buffer_ref(avpkt->buf);
> +
> +    if (!frame->buf[0]) {
> +        return AVERROR(ENOMEM);
> +    }
> +
> +    return av_image_fill_arrays(frame->data, frame->linesize, avpkt->data,
> +                               avctx->pix_fmt, avctx->width, avctx->height, 1);
> +}
> +
> +static int float2planes(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt){
> +
> +    int ret, x, y, lw;
> +    const size_t sof = 4;
> +
> +    ret = ff_thread_get_buffer(avctx, frame, 0);
> +    if (ret < 0)
> +        return ret;
> +
> +    lw = frame->width;
> +
> +    for(y = 0; y < frame->height; y++){
> +        for(x = 0; x < frame->width; x++){
> +            memcpy(&frame->data[2][sof*(lw*y + x)], &pkt->data[sof* 3*(lw*y + x)], sof);
> +            memcpy(&frame->data[0][sof*(lw*y + x)], &pkt->data[sof*(3*(lw*y + x) + 1)], sof);
> +            memcpy(&frame->data[1][sof*(lw*y + x)], &pkt->data[sof*(3*(lw*y + x) + 2)], sof);
> +        }
> +    }
> +    return pkt->size;
> +}
> +
> +static int half_add_alpha(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt){
> +
> +    /* ffmpeg doesn't provide RGB half bit depth without alpha channel right now
> +     * we simply add an opaque alpha layer as workaround */
> +
> +    int ret, x, y, lw;
> +    const size_t soh = 2;
> +    const uint16_t opaque = 0x3c00;
> +
> +    ret = ff_thread_get_buffer(avctx, frame, 0);
> +    if (ret < 0)
> +        return ret;
> +
> +    lw = frame->width;
> +
> +    for(y = 0; y < frame->height; y++){
> +        for(x = 0; x < frame->width; x++){
> +            memcpy(&frame->data[0][soh*4*(lw*y + x)], &pkt->data[soh*3*(lw*y + x)], soh*3);
> +            memcpy(&frame->data[0][soh*(4*(lw*y + x) + 3)], &opaque, soh);
> +        }
> +    }
> +    return pkt->size;
> +}
> +
> +/* DNxUncompressed utilizes a very dense bitpack representation of 10bit and 12bit pixel data.
> +
> +Lines of Image data, which look like in their ordinary 8bit counterpart, contain the most
> +significant upper bits of the pixel data. These sections alternate with shorter segments in
> +which the complementary least significant bits of information get packed in a gapless sequence.
> +
> ++----------------------+ +----------------------+ +------------------------+ +----------~
> +|  8 m.s.bits of R[1]  | |  8 m.s.bits of G[1]  | |  8 m.s.bits of B[1]    | | msb R[2]    ... one line
> ++----------------------+ +----------------------+ +------------------------+ +----------~
> +
> ++---------------------------------------------------------------+  +-----------~
> +| +------------+ +------------+ +------------+ +--------------+ |  | +--------~
> +| | 2 lsb R[1] | | 2 lsb G[1] | | 2 lsb B[1] | | 2 lsb R[2]   | |  | | G[2]lsb    ... LSB bits for line
> +| +------------+ +------------+ +------------+ +--------------+ |  | +--------~
> ++---------------------------- one byte ------------------------ +  +-----------~
> +
> +next line of MSB bytes...   */
> +
> +static int unpack_rg10(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt){
> +
> +    int ret, x, y, lw, msp, pack, lsp, p_off;
> +    uint16_t r,g,b;
> +
> +    if (avctx->width % 4){
> +        av_log(0, AV_LOG_ERROR,
> +        "Image width has to be dividable by 4 for 10bit RGB DNxUncompressed!\n");
> +        return AVERROR_EXIT;
> +    }
> +
> +    ret = ff_thread_get_buffer(avctx, frame, 0);
> +    if (ret < 0)
> +        return ret;
> +
> +    lw = frame->width;
> +
> +    for(y = 0; y < frame->height; y++){
> +        for(x = 0; x < frame->width; x++){
> +            msp = pkt->data[y*3*(lw + lw/4) + 3*x];
> +            p_off = y*(3*(lw + lw/4)) + 3*lw + 3*x/4;
> +            pack = pkt->data[p_off];
> +            lsp = (pack >> (3*x%4)*2) & 0x3;
> +            r = (msp << 2) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, "r: %04x, %02x, %02x, %02x, %d\n",
> +            //    r, msp, lsp, pack, p_off);
> +
> +            msp = pkt->data[y*3*(lw + lw/4) + 3*x + 1];
> +            p_off = y*(3*(lw + lw/4)) + 3*lw + (3*x+1)/4;
> +            pack = pkt->data[p_off];
> +            lsp = (pack >> ((3*x+1)%4)*2) & 0x3;
> +            g = (msp << 2) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, "g: %04x, %02x, %02x, %02x, %d\n",
> +            //    g, msp, lsp, pack, p_off);
> +
> +            msp = pkt->data[y*3*(lw + lw/4) + 3*x + 2];
> +            p_off = y*(3*(lw + lw/4)) + 3*lw + (3*x+2)/4;
> +            pack = pkt->data[p_off];
> +            lsp = (pack >> ((3*x+2)%4)*2) & 0x3;
> +            b = (msp << 2) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, "b: %04x, %02x, %02x, %02x, %d\n\n",
> +            //    b, msp, lsp, pack, p_off);
> +
> +            memcpy(&frame->data[2][2*(y*lw + x)], &r, 2);
> +            memcpy(&frame->data[0][2*(y*lw + x)], &g, 2);
> +            memcpy(&frame->data[1][2*(y*lw + x)], &b, 2);
> +        }
> +    }
> +    return pkt->size;
> +}
> +
> +static int unpack_rg12(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt){
> +
> +    int ret, x, y, lw, msp, pack, lsp, p_off;
> +    uint16_t r,g,b;
> +
> +    if (avctx->width % 2){
> +        av_log(0, AV_LOG_ERROR,
> +        "Image width has to be dividable by 2 for 12bit RGB DNxUncompressed!\n");
> +        return AVERROR_EXIT;
> +    }
> +
> +    ret = ff_thread_get_buffer(avctx, frame, 0);
> +    if (ret < 0)
> +        return ret;
> +
> +    lw = frame->width;
> +
> +    for(y = 0; y < frame->height; y++){
> +        for(x = 0; x < frame->width; x++){
> +            msp = pkt->data[y*3*(lw + lw/2) + 3*x];
> +            p_off = y*(3*(lw + lw/2)) + 3*lw + 3*x/2;
> +            pack = pkt->data[p_off];
> +            lsp = (pack >> (3*x%2)*4) & 0xf;
> +            r = (msp << 4) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, "r: %04x, %02x, %02x, %02x, %d\n",
> +            //    r, msp, lsp, pack, p_off);
> +
> +            msp = pkt->data[y*3*(lw + lw/2) + 3*x + 1];
> +            p_off =y*(3*(lw + lw/2)) + 3*lw + (3*x+1)/2;
> +            pack = pkt->data[p_off];
> +            lsp = (pack >> ((3*x+1)%2)*4) & 0xf;
> +            g = (msp << 4) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, "g: %04x, %02x, %02x, %02x, %d\n",
> +            //    g, msp, lsp, pack, p_off);
> +
> +            msp = pkt->data[y*3*(lw + lw/2) + 3*x + 2];
> +            p_off = y*(3*(lw + lw/2)) + 3*lw + (3*x+2)/2;
> +            pack = pkt->data[p_off];
> +            lsp = (pack >> ((3*x+2)%2)*4) & 0xf;
> +            b = (msp << 4) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, "b: %04x, %02x, %02x, %02x, %d\n\n",
> +            //    b, msp, lsp, pack, p_off);
> +
> +            memcpy(&frame->data[2][2*(y*lw + x)], &r, 2);
> +            memcpy(&frame->data[0][2*(y*lw + x)], &g, 2);
> +            memcpy(&frame->data[1][2*(y*lw + x)], &b, 2);
> +        }
> +    }
> +    return pkt->size;
> +}
> +
> +
> +static int unpack_y210(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt){
> +
> +    int ret, x, y, lw, msp, pack, lsp, p_off;
> +    uint16_t y1, y2, u, v;
> +
> +    if (avctx->width % 2){
> +        av_log(0, AV_LOG_ERROR,
> +        "Image width has to be dividable by 2 for 10bit YUV 4:2:2 DNxUncompressed!\n");
> +        return AVERROR_EXIT;
> +    }
> +
> +    ret = ff_thread_get_buffer(avctx, frame, 0);
> +    if (ret < 0)
> +        return ret;
> +
> +    lw = frame->width;
> +
> +    for(y = 0; y < frame->height; y++){
> +        for(x = 0; x < frame->width; x += 2){
> +
> +            p_off = y*(2*(lw + lw/4)) + 2*lw + x/2;
> +            pack = pkt->data[p_off];
> +
> +            msp = pkt->data[y*2*(lw + lw/4) + 2*x];
> +            lsp = pack & 0x3;
> +            u = (msp << 2) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, " u: %04x, %02x, %02x, %02x, %d\n",
> +            //    u, msp, lsp, pack, p_off);
> +
> +            msp = pkt->data[y*2*(lw + lw/4) + 2*x + 1];
> +            lsp = (pack >> 2) & 0x3;
> +            y1 = (msp << 2) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, "y1: %04x, %02x, %02x, %02x, %d\n",
> +            //    y1, msp, lsp, pack, p_off);
> +
> +            msp = pkt->data[y*2*(lw + lw/4) + 2*x + 2];
> +            lsp = (pack >> 4) & 0x3;
> +            v = (msp << 2) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, " v: %04x, %02x, %02x, %02x, %d\n",
> +            //    v, msp, lsp, pack, p_off);
> +
> +            msp = pkt->data[y*2*(lw + lw/4) + 2*x + 3];
> +            lsp = (pack >> 6) & 0x3;
> +            y2 = (msp << 2) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, "y2: %04x, %02x, %02x, %02x, %d\n\n",
> +            //    y2, msp, lsp, pack, p_off);
> +
> +            memcpy(&frame->data[0][2*(y*lw + x)], &y1, 2);
> +            memcpy(&frame->data[0][2*(y*lw + x+1)], &y2, 2);
> +            memcpy(&frame->data[1][2*(y*lw/2 + x/2)], &u, 2);
> +            memcpy(&frame->data[2][2*(y*lw/2 + x/2)], &v, 2);
> +        }
> +    }
> +    return pkt->size;
> +}
> +
> +
> +static int unpack_y212(AVCodecContext *avctx, AVFrame *frame, const AVPacket *pkt){
> +
> +    int ret, x, y, lw, msp, pack, lsp, p_off;
> +    uint16_t y1, y2, u, v;
> +
> +    if (avctx->width % 2){
> +        av_log(0, AV_LOG_ERROR,
> +        "Image width has to be dividable by 2 for 12bit YUV 4:2:2 DNxUncompressed!\n");
> +        return AVERROR_EXIT;
> +    }
> +
> +    ret = ff_thread_get_buffer(avctx, frame, 0);
> +    if (ret < 0)
> +        return ret;
> +
> +    lw = frame->width;
> +
> +    for(y = 0; y < frame->height; y++){
> +        for(x = 0; x < frame->width; x += 2){
> +
> +            p_off = y*(2*(lw + lw/2)) + 2*lw + x;
> +            pack = pkt->data[p_off];
> +
> +            msp = pkt->data[y*2*(lw + lw/2) + 2*x];
> +            lsp = pack & 0xf;
> +            u = (msp << 4) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, " u: %04x, %02x, %02x, %02x, %d\n",
> +            //    u, msp, lsp, pack, p_off);
> +
> +            msp = pkt->data[y*2*(lw + lw/2) + 2*x + 1];
> +            lsp = (pack >> 4) & 0xf;
> +            y1 = (msp << 4) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, "y1: %04x, %02x, %02x, %02x, %d\n",
> +            //    y1, msp, lsp, pack, p_off);
> +
> +            p_off = y*(2*(lw + lw/2)) + 2*lw + x+1;
> +            pack = pkt->data[p_off];
> +
> +            msp = pkt->data[y*2*(lw + lw/2) + 2*x + 2];
> +            lsp = pack & 0xf;
> +            v = (msp << 4) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, " v: %04x, %02x, %02x, %02x, %d\n",
> +            //    v, msp, lsp, pack, p_off);
> +
> +            msp = pkt->data[y*2*(lw + lw/2) + 2*x + 3];
> +            lsp = (pack >> 4) & 0xf;
> +            y2 = (msp << 4) + lsp;
> +            // av_log(0, AV_LOG_DEBUG, "y2: %04x, %02x, %02x, %02x, %d\n\n",
> +            //    y2, msp, lsp, pack, p_off);
> +
> +            memcpy(&frame->data[0][2*(y*lw + x)], &y1, 2);
> +            memcpy(&frame->data[0][2*(y*lw + x+1)], &y2, 2);
> +            memcpy(&frame->data[1][2*(y*lw/2 + x/2)], &u, 2);
> +            memcpy(&frame->data[2][2*(y*lw/2 + x/2)], &v, 2);
> +        }
> +    }
> +    return pkt->size;
> +}
> +
> +
> +static int dnxuc_decode_frame(AVCodecContext *avctx, AVFrame *frame,
> +                             int *got_frame, AVPacket *avpkt) {
> +
> +    char fourcc_buf[5];
> +    int ret;
> +
> +    switch (avctx->codec_tag) {
> +        case MKTAG('r','g','0','8'):
> +            avctx->pix_fmt = AV_PIX_FMT_RGB24;
> +            ret = pass_though(avctx, frame, avpkt);
> +            break;
> +        case MKTAG('r','g','1','0'):
> +            avctx->pix_fmt = AV_PIX_FMT_GBRP10LE;
> +            ret = unpack_rg10(avctx, frame, avpkt);
> +            break;
> +        case MKTAG('r','g','1','2'):
> +            avctx->pix_fmt = AV_PIX_FMT_GBRP12LE;
> +            ret = unpack_rg12(avctx, frame, avpkt);
> +            break;
> +        case MKTAG(' ','r','g','h'):
> +            avctx->pix_fmt = AV_PIX_FMT_RGBAF16LE;
> +            ret = half_add_alpha(avctx, frame, avpkt);
> +            break;
> +        case MKTAG(' ','r','g','f'):
> +            avctx->pix_fmt = AV_PIX_FMT_GBRPF32LE;
> +            ret = float2planes(avctx, frame, avpkt);
> +            break;
> +
> +        case MKTAG('y','2','0','8'):
> +            avctx->pix_fmt = AV_PIX_FMT_UYVY422;
> +            ret = pass_though(avctx, frame, avpkt);
> +            break;
> +        case MKTAG('y','2','1','0'):
> +            avctx->pix_fmt = AV_PIX_FMT_YUV422P10LE;
> +            ret = unpack_y210(avctx, frame, avpkt);
> +            break;
> +        case MKTAG('y','2','1','2'):
> +            avctx->pix_fmt = AV_PIX_FMT_YUV422P12LE;
> +            ret = unpack_y212(avctx, frame, avpkt);
> +            break;
> +
> +        default:
> +            av_fourcc_make_string(fourcc_buf, avctx->codec_tag);
> +            av_log(0, AV_LOG_ERROR,
> +            "Unsupported DNxUncompressed pixel format variant: '%s'\n",
> +            fourcc_buf);
> +            return AVERROR(ENOSYS);
> +    }
> +
> +    if (ret < 0) {
> +        av_buffer_unref(&frame->buf[0]);
> +        return ret;
> +    }
> +
> +    *got_frame = 1;
> +
> +    return avpkt->size;
> +}
> +
> +const AVCodecParser ff_dnxuc_parser = {
> +    .codec_ids      = { AV_CODEC_ID_DNXUC },
> +    .priv_data_size = sizeof(DNxUcParseContext),
> +    .parser_parse   = dnxuc_parse,
> +};
> +
> +const FFCodec ff_dnxuc_decoder = {
> +    .p.name         = "dnxuc",
> +    CODEC_LONG_NAME("DNxUncompressed (SMPTE RDD 50)"),
> +    .p.type         = AVMEDIA_TYPE_VIDEO,
> +    .p.id             = AV_CODEC_ID_DNXUC,
> +    .init           = dnxuc_decode_init,
> +    FF_CODEC_DECODE_CB(dnxuc_decode_frame),
> +    .p.capabilities = AV_CODEC_CAP_FRAME_THREADS,
> +};
> diff --git a/libavcodec/parsers.c b/libavcodec/parsers.c
> index 5128009..8bfd2db 100644
> --- a/libavcodec/parsers.c
> +++ b/libavcodec/parsers.c
> @@ -35,6 +35,7 @@ extern const AVCodecParser ff_cri_parser;
> extern const AVCodecParser ff_dca_parser;
> extern const AVCodecParser ff_dirac_parser;
> extern const AVCodecParser ff_dnxhd_parser;
> +extern const AVCodecParser ff_dnxuc_parser;
> extern const AVCodecParser ff_dolby_e_parser;
> extern const AVCodecParser ff_dpx_parser;
> extern const AVCodecParser ff_dvaudio_parser;
> diff --git a/libavcodec/version.c b/libavcodec/version.c
> index 27f9432..c3b576a 100644
> --- a/libavcodec/version.c
> +++ b/libavcodec/version.c
> @@ -31,7 +31,7 @@ const char av_codec_ffversion[] = "FFmpeg version " FFMPEG_VERSION;
>
> unsigned avcodec_version(void)
> {
> -    static_assert(AV_CODEC_ID_LEAD         ==   269 &&
> +    static_assert(AV_CODEC_ID_LEAD         ==   270 &&
>                   AV_CODEC_ID_PCM_SGA      == 65572 &&
>                   AV_CODEC_ID_ADPCM_XMD    == 69683 &&
>                   AV_CODEC_ID_CBD2_DPCM    == 81928 &&
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 8b53586..da2264a 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,7 +29,7 @@
>
> #include "version_major.h"
>
> -#define LIBAVCODEC_VERSION_MINOR  11
> +#define LIBAVCODEC_VERSION_MINOR  12
> #define LIBAVCODEC_VERSION_MICRO 100
>
> #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> diff --git a/libavformat/mxf.c b/libavformat/mxf.c
> index a73e40e..35fb73e 100644
> --- a/libavformat/mxf.c
> +++ b/libavformat/mxf.c
> @@ -61,6 +61,7 @@ const MXFCodecUL ff_mxf_codec_uls[] = {
>     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x71,0x00,0x00,0x00 }, 13,      AV_CODEC_ID_DNXHD }, /* SMPTE VC-3/DNxHD */
>     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x03,0x02,0x00,0x00 }, 14,      AV_CODEC_ID_DNXHD }, /* SMPTE VC-3/DNxHD */
>     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x04,0x01,0x00 }, 16,      AV_CODEC_ID_DNXHD }, /* SMPTE VC-3/DNxHD Legacy Avid Media Composer MXF */
> +    { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0E,0x04,0x02,0x01,0x02,0x1E,0x01,0x00 }, 16,      AV_CODEC_ID_DNXUC }, /* DNxUncompressed/SMPTE RDD 50 */
>     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x01,0x32,0x00,0x00 }, 14,       AV_CODEC_ID_H264 }, /* H.264/MPEG-4 AVC Intra */
>     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x01,0x31,0x11,0x01 }, 14,       AV_CODEC_ID_H264 }, /* H.264/MPEG-4 AVC SPS/PPS in-band */
>     { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x01,0x01,0x02,0x02,0x01 }, 16,       AV_CODEC_ID_V210 }, /* V210 */
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index ac63c0d..dc07e27 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -208,13 +208,18 @@ typedef struct MXFDescriptor {
>     int field_dominance;
>     int channels;
>     int bits_per_sample;
> +    int padding_bits;
>     int64_t duration; /* ContainerDuration optional property */
>     unsigned int component_depth;
> +    unsigned int color_siting;
>     unsigned int black_ref_level;
>     unsigned int white_ref_level;
>     unsigned int color_range;
>     unsigned int horiz_subsampling;
>     unsigned int vert_subsampling;
> +    unsigned int reversed_byte_order;
> +    unsigned int image_start_offset;
> +    unsigned int image_end_offset;
>     UID *file_descriptors_refs;
>     int file_descriptors_count;
>     UID *sub_descriptors_refs;
> @@ -1364,6 +1369,12 @@ static int mxf_read_generic_descriptor(void *arg, AVIOContext *pb, int tag, int
>     case 0x3212:
>         descriptor->field_dominance = avio_r8(pb);
>         break;
> +    case 0x3213:
> +        descriptor->image_start_offset = avio_rb32(pb);
> +        break;
> +    case 0x3214:
> +        descriptor->image_end_offset = avio_rb32(pb);
> +        break;
>     case 0x3219:
>         avio_read(pb, descriptor->color_primaries_ul, 16);
>         break;
> @@ -1376,6 +1387,9 @@ static int mxf_read_generic_descriptor(void *arg, AVIOContext *pb, int tag, int
>     case 0x3302:
>         descriptor->horiz_subsampling = avio_rb32(pb);
>         break;
> +    case 0x3303:
> +        descriptor->color_siting = avio_r8(pb);
> +        break;
>     case 0x3304:
>         descriptor->black_ref_level = avio_rb32(pb);
>         break;
> @@ -1385,9 +1399,15 @@ static int mxf_read_generic_descriptor(void *arg, AVIOContext *pb, int tag, int
>     case 0x3306:
>         descriptor->color_range = avio_rb32(pb);
>         break;
> +    case 0x3307:
> +        descriptor->padding_bits = avio_rb16(pb);
> +        break;
>     case 0x3308:
>         descriptor->vert_subsampling = avio_rb32(pb);
>         break;
> +    case 0x330B:
> +        descriptor->reversed_byte_order = avio_r8(pb);
> +        break;
>     case 0x3D03:
>         descriptor->sample_rate.num = avio_rb32(pb);
>         descriptor->sample_rate.den = avio_rb32(pb);
> @@ -1597,6 +1617,7 @@ static const MXFCodecUL mxf_picture_essence_container_uls[] = {
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c,0x01,0x00 }, 14,   AV_CODEC_ID_JPEG2000, NULL, 14, J2KWrap },
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x10,0x60,0x01 }, 14,       AV_CODEC_ID_H264, NULL, 15 }, /* H.264 */
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x11,0x01,0x00 }, 14,      AV_CODEC_ID_DNXHD, NULL, 14 }, /* VC-3 */
> +    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x1e,0x01,0x00 }, 14,      AV_CODEC_ID_DNXUC, NULL, 14 }, /* DNxUncompressed / SMPTE RDD 50 */
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x12,0x01,0x00 }, 14,        AV_CODEC_ID_VC1, NULL, 14 }, /* VC-1 */
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x14,0x01,0x00 }, 14,       AV_CODEC_ID_TIFF, NULL, 14 }, /* TIFF */
>     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x15,0x01,0x00 }, 14,      AV_CODEC_ID_DIRAC, NULL, 14 }, /* VC-2 */
> -- 
> 2.45.2
>
> _______________________________________________
> 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".
>

Thanks for this careful review.

On 09.09.24 12:56, Anton Khirnov wrote:
> Quoting Martin Schitter (2024-09-08 20:41:38)
>> diff --git a/libavcodec/dnxucdec.c b/libavcodec/dnxucdec.c
>> new file mode 100644
>> index 0000000..455c374
>> --- /dev/null
>> +++ b/libavcodec/dnxucdec.c
>> @@ -0,0 +1,495 @@
>> +/*
>> + * Avid DNxUncomressed / SMPTE RDD 50 demuxer
> 
> This says it's a demuxer, while it's implemented as a decoder.

Fixed.

> I'm also wondering if this shouldn't be handled as demuxer exporting
> raw video, at least in some of cases if not all.

When I started the work, I also thought, it will not require much more 
than minimal modifications of MXF related raw data transport details, 
but during reverse engineering the actually used dense bitpacking turned 
out to be more complicated.

The codec section should fit rather well for this component, although 
large parts of it could be also categorized just as bitstream filter.

>> +typedef struct DNxUcParseContext {
>> +    uint32_t fourcc_tag;
>> +    uint32_t width;
>> +    uint32_t height;
>> +    uint32_t nr_bytes;
>> +} DNxUcParseContext;
> 
> Parser should be in its own file, as it can be enabled or disabled
> independently from the encoder.

O.k. that's possible.

Done.

> Opening brace of a function body should be on its own line.

Fixed.

>> +            av_log(0, AV_LOG_ERROR, "can't read DNxUncompressed metadata.\n");
> 
> av_log() should always get a proper logging context, avctx in this case

Fixed.

>> +            *poutbuf_size = 0;
>> +            return buf_size;
>> +    }
>> +
>> +    pc->fourcc_tag = *(uint32_t*)(buf+24);
>> +    pc->width =  *(uint32_t*)(buf+16);
>> +    pc->height =  *(uint32_t*)(buf+20);
>> +    pc->nr_bytes = *(uint32_t*)(buf+29) - 8;
> 
> Use AV_RL32()

Done.

>> +    ret = ff_thread_get_buffer(avctx, frame, 0);
>> +    if (ret < 0)
>> +        return ret;
> 
> This call should not be duplicated in every handler.

Eliminated by some refactoring and an additional helper function.

>> +    if (avctx->width % 4){
>> +        av_log(0, AV_LOG_ERROR,
>> +        "Image width has to be dividable by 4 for 10bit RGB DNxUncompressed!\n");
>> +        return AVERROR_EXIT;
>> +    }
> 
> These checks can be done once during init.

Yes -- A line width check for a multiple of 2 is now done only once for 
all yuv4:2:2 compression variants. This is more standard compliant and 
should work for all implemented format handlers.

>> +    ret = ff_thread_get_buffer(avctx, frame, 0);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    lw = frame->width;
>> +
>> +    for(y = 0; y < frame->height; y++){
>> +        for(x = 0; x < frame->width; x++){
>> +            msp = pkt->data[y*3*(lw + lw/4) + 3*x];
> 
> Does anything guarantee the packet is large enough?

I added another packet size check, which should calculate the needed 
size of data for this kind of very dense bit packed input.

I hope you like the chosen solutions.

Martin

On 09.09.24 12:42, Anton Khirnov wrote:
> Quoting Martin Schitter (2024-09-08 20:41:38)
>> I'm still unsure about my solution of adding the codec_id entries and
>> increasing the lead-position value, as criticized by Andreas Rheinhard,
>> but I don't see any other way to make it better.
> 
> I think you're misunderstanding the meaning of AV_CODEC_ID_LEAD - it's
> not in any way special, it's just the ID of a codec called "LEAD MCMP".
> You should add your new codec ID immediately after it.

Thanks for this hint!

Now I finally got it. :)

Martin