Re: [FFmpeg-devel] [PATCH 2/6] lavf: APV demuxer

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 6221 bytes --]

On Sat, Apr 19, 2025 at 08:07:00PM +0100, Mark Thompson wrote:
> Demuxes raw streams as defined in draft spec section 10.2.
> ---
>  libavformat/Makefile     |   1 +
>  libavformat/allformats.c |   1 +
>  libavformat/apvdec.c     | 245 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 247 insertions(+)
>  create mode 100644 libavformat/apvdec.c
> 
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index a94ac66e7e..ef96c2762e 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -119,6 +119,7 @@ OBJS-$(CONFIG_APTX_DEMUXER)              += aptxdec.o
>  OBJS-$(CONFIG_APTX_MUXER)                += rawenc.o
>  OBJS-$(CONFIG_APTX_HD_DEMUXER)           += aptxdec.o
>  OBJS-$(CONFIG_APTX_HD_MUXER)             += rawenc.o
> +OBJS-$(CONFIG_APV_DEMUXER)               += apvdec.o
>  OBJS-$(CONFIG_AQTITLE_DEMUXER)           += aqtitledec.o subtitles.o
>  OBJS-$(CONFIG_ARGO_ASF_DEMUXER)          += argo_asf.o
>  OBJS-$(CONFIG_ARGO_ASF_MUXER)            += argo_asf.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 445f13f42a..90a4fe64ec 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -72,6 +72,7 @@ extern const FFInputFormat  ff_aptx_demuxer;
>  extern const FFOutputFormat ff_aptx_muxer;
>  extern const FFInputFormat  ff_aptx_hd_demuxer;
>  extern const FFOutputFormat ff_aptx_hd_muxer;
> +extern const FFInputFormat  ff_apv_demuxer;
>  extern const FFInputFormat  ff_aqtitle_demuxer;
>  extern const FFInputFormat  ff_argo_asf_demuxer;
>  extern const FFOutputFormat ff_argo_asf_muxer;
> diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c
> new file mode 100644
> index 0000000000..008cbef708
> --- /dev/null
> +++ b/libavformat/apvdec.c
> @@ -0,0 +1,245 @@
> +/*
> + * 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 "libavcodec/apv.h"

missing file



> +#include "libavcodec/get_bits.h"
> +#include "avformat.h"
> +#include "avio_internal.h"
> +#include "demux.h"
> +#include "internal.h"
> +
> +
> +#define APV_TAG MKBETAG('a', 'P', 'v', '1')
> +
> +typedef struct APVHeaderInfo {
> +    uint8_t  pbu_type;
> +    uint16_t group_id;
> +
> +    uint8_t  profile_idc;
> +    uint8_t  level_idc;
> +    uint8_t  band_idc;
> +
> +    uint32_t frame_width;
> +    uint32_t frame_height;
> +
> +    uint8_t  chroma_format_idc;
> +    uint8_t  bit_depth_minus8;
> +
> +    enum AVPixelFormat pixel_format;
> +} APVHeaderInfo;
> +
> +static const enum AVPixelFormat apv_format_table[5][5] = {
> +    { AV_PIX_FMT_GRAY8,    AV_PIX_FMT_GRAY10,     AV_PIX_FMT_GRAY12,     AV_PIX_FMT_GRAY14, AV_PIX_FMT_GRAY16 },
> +    { 0 }, // 4:2:0 is not valid.
> +    { AV_PIX_FMT_YUV422P,  AV_PIX_FMT_YUV422P10,  AV_PIX_FMT_YUV422P12,  AV_PIX_FMT_GRAY14, AV_PIX_FMT_YUV422P16 },
> +    { AV_PIX_FMT_YUV444P,  AV_PIX_FMT_YUV444P10,  AV_PIX_FMT_YUV444P12,  AV_PIX_FMT_GRAY14, AV_PIX_FMT_YUV444P16 },
> +    { AV_PIX_FMT_YUVA444P, AV_PIX_FMT_YUVA444P10, AV_PIX_FMT_YUVA444P12, AV_PIX_FMT_GRAY14, AV_PIX_FMT_YUVA444P16 },
> +};
> +

> +static int apv_extract_header_info(APVHeaderInfo *info,
> +                                   GetBitContext *gbc)
> +{
> +    int zero, bit_depth_index;
> +
> +    info->pbu_type = get_bits(gbc, 8);
> +    info->group_id = get_bits(gbc, 16);
> +
> +    zero = get_bits(gbc, 8);
> +    if (zero != 0)
> +        return 0;

this should use negative AVERROR* for errors for consistency



> +
> +    if (info->pbu_type != APV_PBU_PRIMARY_FRAME)
> +        return 0;
> +
> +    info->profile_idc = get_bits(gbc, 8);
> +    info->level_idc   = get_bits(gbc, 8);
> +    info->band_idc    = get_bits(gbc, 3);
> +
> +    zero = get_bits(gbc, 5);
> +    if (zero != 0)
> +        return 0;
> +

> +    info->frame_width  = get_bits(gbc, 24);
> +    info->frame_height = get_bits(gbc, 24);

frame_width and frame_height would be better as int instead of uint32_t
given its only 24bit

its also more consistent
git grep 'int *width' | wc
   1935   15372  180444

git grep 'uint32_t *width' | wc
     15      62     740



[...]
> +static int apv_probe(const AVProbeData *p)
> +{
> +    GetBitContext gbc;
> +    APVHeaderInfo header;
> +    uint32_t au_size, tag, pbu_size;
> +    int score = AVPROBE_SCORE_EXTENSION + 1;
> +    int ret;
> +
> +    init_get_bits8(&gbc, p->buf, p->buf_size);
> +
> +    au_size = get_bits_long(&gbc, 32);
> +    if (au_size < 16) {
> +        // Too small.
> +        return 0;
> +    }
> +    // The spec doesn't have this tag, but the reference software and
> +    // all current files do.  Treat it as optional and skip if present,
> +    // but if it is there then this is definitely an APV file.
> +    tag = get_bits_long(&gbc, 32);
> +    if (tag == APV_TAG) {
> +        pbu_size = get_bits_long(&gbc, 32);
> +        score = AVPROBE_SCORE_MAX;
> +    } else {
> +        pbu_size = tag;
> +    }
> +    if (pbu_size < 16) {
> +        // Too small.
> +        return 0;
> +    }
> +
> +    ret = apv_extract_header_info(&header, &gbc);

something somewhere should check buf_size or size in gbc
otherwise a truncated header would be accepted

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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