Re: [FFmpeg-devel] [PATCH] add prores bitstream demuxer and muxer

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

hung kuishing:
> ---
>  libavcodec/Makefile        |  1 +
>  libavcodec/parsers.c       |  1 +
>  libavcodec/prores_parser.c | 91 ++++++++++++++++++++++++++++++++++++++
>  libavformat/Makefile       |  2 +
>  libavformat/allformats.c   |  2 +
>  libavformat/proresdec.c    | 62 ++++++++++++++++++++++++++
>  libavformat/rawenc.c       | 13 ++++++
>  7 files changed, 172 insertions(+)
>  create mode 100644 libavcodec/prores_parser.c
>  create mode 100644 libavformat/proresdec.c
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 1b0226c089..b6ebbfb340 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -1214,6 +1214,7 @@ OBJS-$(CONFIG_WEBP_PARSER)             += webp_parser.o
>  OBJS-$(CONFIG_XBM_PARSER)              += xbm_parser.o
>  OBJS-$(CONFIG_XMA_PARSER)              += xma_parser.o
>  OBJS-$(CONFIG_XWD_PARSER)              += xwd_parser.o
> +OBJS-$(CONFIG_PRORES_PARSER)           += prores_parser.o
>  
>  # bitstream filters
>  OBJS-$(CONFIG_AAC_ADTSTOASC_BSF)          += aac_adtstoasc_bsf.o
> diff --git a/libavcodec/parsers.c b/libavcodec/parsers.c
> index 285f81a901..ff7c15b7d1 100644
> --- a/libavcodec/parsers.c
> +++ b/libavcodec/parsers.c
> @@ -80,6 +80,7 @@ extern const AVCodecParser ff_webp_parser;
>  extern const AVCodecParser ff_xbm_parser;
>  extern const AVCodecParser ff_xma_parser;
>  extern const AVCodecParser ff_xwd_parser;
> +extern const AVCodecParser ff_prores_parser;

These lists are supposed to be sorted alphabetically.

>  
>  #include "libavcodec/parser_list.c"
>  
> diff --git a/libavcodec/prores_parser.c b/libavcodec/prores_parser.c
> new file mode 100644
> index 0000000000..4b50147768
> --- /dev/null
> +++ b/libavcodec/prores_parser.c
> @@ -0,0 +1,91 @@
> +/*
> + * ProRes bitstream parser
> + * Copyright (c) 2023 clarkh <hungkuishing@outlook.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 "parser.h"
> +#include "libavutil/intreadwrite.h"
> +
> +static int prores_find_frame_end(ParseContext *pc, const uint8_t *buf, int buf_size)
> +{
> +    int pic_found  = pc->frame_start_found;
> +    uint32_t state = pc->state;
> +    int cur = 0;
> +
> +    int flag = AV_RB32("icpf");

MKBETAG; also use uint32_t for flag.

> +    if (!pic_found) {
> +        for (; cur < buf_size; cur++) {
> +            state = (state<<8) | buf[cur];
> +            if (state == flag){
> +                ++cur;
> +                pic_found = 1;
> +                break;
> +            }
> +        }
> +    }
> +
> +    if (pic_found) {
> +        if (!buf_size)
> +            return END_NOT_FOUND;
> +        for (; cur < buf_size; ++cur) {
> +            state = (state << 8) | buf[cur];
> +            if (state == flag) {
> +                pc->frame_start_found = 0;
> +                pc->state = -1;
> +                return cur - 7;
> +            }
> +        }
> +    }
> +
> +    pc->frame_start_found = pic_found;
> +    pc->state = state;
> +
> +    return END_NOT_FOUND;
> +}
> +
> +static int prores_parse(AVCodecParserContext *s, AVCodecContext *avctx,
> +                      const uint8_t **poutbuf, int *poutbuf_size,
> +                      const uint8_t *buf, int buf_size)
> +{
> +    ParseContext *pc = s->priv_data;
> +    int next;
> +
> +    if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) {
> +        next = buf_size;
> +    } else {
> +        next = prores_find_frame_end(pc, buf, buf_size);
> +        if (ff_combine_frame(pc, next, &buf, &buf_size) < 0) {
> +            *poutbuf = NULL;
> +            *poutbuf_size = 0;
> +            return buf_size;
> +        }
> +    }
> +
> +    *poutbuf = buf;
> +    *poutbuf_size = buf_size;
> +
> +    return next;
> +}
> +
> +const AVCodecParser ff_prores_parser = {
> +    .codec_ids      = { AV_CODEC_ID_PRORES },
> +    .priv_data_size = sizeof(ParseContext),
> +    .parser_parse   = prores_parse,
> +    .parser_close   = ff_parse_close
> +};
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 4cb00f8700..a7f265252d 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -639,6 +639,8 @@ OBJS-$(CONFIG_XWMA_DEMUXER)              += xwma.o
>  OBJS-$(CONFIG_YOP_DEMUXER)               += yop.o
>  OBJS-$(CONFIG_YUV4MPEGPIPE_DEMUXER)      += yuv4mpegdec.o
>  OBJS-$(CONFIG_YUV4MPEGPIPE_MUXER)        += yuv4mpegenc.o
> +OBJS-$(CONFIG_PRORES_DEMUXER)            += proresdec.o rawdec.o
> +OBJS-$(CONFIG_PRORES_MUXER)              += rawenc.o
>  
>  # external library muxers/demuxers
>  OBJS-$(CONFIG_AVISYNTH_DEMUXER)          += avisynth.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 6324952bd2..89533eb686 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -520,6 +520,8 @@ extern const AVInputFormat  ff_xwma_demuxer;
>  extern const AVInputFormat  ff_yop_demuxer;
>  extern const AVInputFormat  ff_yuv4mpegpipe_demuxer;
>  extern const FFOutputFormat ff_yuv4mpegpipe_muxer;
> +extern const AVInputFormat  ff_prores_demuxer;
> +extern const FFOutputFormat ff_prores_muxer;
>  /* image demuxers */
>  extern const AVInputFormat  ff_image_bmp_pipe_demuxer;
>  extern const AVInputFormat  ff_image_cri_pipe_demuxer;
> diff --git a/libavformat/proresdec.c b/libavformat/proresdec.c
> new file mode 100644
> index 0000000000..11541f8cd3
> --- /dev/null
> +++ b/libavformat/proresdec.c
> @@ -0,0 +1,62 @@
> +/*
> + * ProRes bitstream demuxer
> + * Copyright (c) 2023 clarkh <hungkuishing@outlook.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 "libavutil/intreadwrite.h"
> +#include "avformat.h"
> +#include "rawdec.h"
> +
> +static int prores_check_frame_header(const uint8_t *buf, const int data_size)
> +{
> +    int hdr_size, width, height;
> +    int version, alpha_info;
> +
> +    hdr_size = AV_RB16(buf);
> +    if (hdr_size < 20)
> +        return AVERROR_INVALIDDATA;
> +
> +    version = buf[3];
> +    if (version > 1)
> +        return AVERROR_INVALIDDATA;
> +
> +    width  = AV_RB16(buf + 8);
> +    height = AV_RB16(buf + 10);
> +    if (!width || !height)
> +        return AVERROR_INVALIDDATA;
> +
> +    alpha_info = buf[17] & 0x0f;
> +    if (alpha_info > 2)
> +        return AVERROR_INVALIDDATA;
> +
> +    return 0;
> +}
> +
> +static int prores_probe(const AVProbeData *p)
> +{
> +    if (p->buf_size < 28 || AV_RL32(p->buf + 4) != AV_RL32("icpf"))

There is no need to use little-endian here (it is unnatural for a
big-endian format anyway). You can e.g. use AV_RN32 for both.

> +        return 0;
> +
> +    if (prores_check_frame_header(p->buf + 8, p->buf_size - 8) < 0)
> +        return 0;
> +
> +    return AVPROBE_SCORE_MAX;
> +}
> +
> +FF_DEF_RAWVIDEO_DEMUXER(prores, "raw ProRes", prores_probe, NULL, AV_CODEC_ID_PRORES)

IIRC ProRes's subblocks have an ISOBMFF-style length-field prefixed to
the block, yet you do not use this and simply return a small amount of
data (RAW_PACKET_SIZE (=1024B) by default) in each packet. This doesn't
seem reasonable.

The parser, too, just ignores this and simply searches for the sync
code. In this case, I am actually wondering whether this is correct at
all: Is it guaranteed that "icpf" can't appear inside one of these
subblocks, causing misparsing with your proposed parser? If it is
length-prefixed, there is no need for escaping the startcode.

> diff --git a/libavformat/rawenc.c b/libavformat/rawenc.c
> index f916db13a2..db7d88e782 100644
> --- a/libavformat/rawenc.c
> +++ b/libavformat/rawenc.c
> @@ -588,3 +588,16 @@ const FFOutputFormat ff_vc1_muxer = {
>      .p.flags           = AVFMT_NOTIMESTAMPS,
>  };
>  #endif
> +
> +#if CONFIG_PRORES_MUXER
> +const FFOutputFormat ff_prores_muxer = {
> +    .p.name            = "prores",
> +    .p.long_name       = NULL_IF_CONFIG_SMALL("raw prores video"),
> +    .p.extensions      = "prores",
> +    .p.audio_codec     = AV_CODEC_ID_NONE,
> +    .p.video_codec     = AV_CODEC_ID_PRORES,
> +    .init              = force_one_stream,
> +    .write_packet      = ff_raw_write_packet,
> +    .p.flags           = AVFMT_NOTIMESTAMPS,
> +};
> +#endif

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