From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <ffmpeg-devel-bounces@ffmpeg.org>
Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100])
by master.gitmailbox.com (Postfix) with ESMTPS id BF2B64D948
for <ffmpegdev@gitmailbox.com>; Mon, 21 Apr 2025 14:59:07 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C28DA687D8B;
Mon, 21 Apr 2025 17:59:03 +0300 (EEST)
Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com
[209.85.128.49])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 53A95687B64
for <ffmpeg-devel@ffmpeg.org>; Mon, 21 Apr 2025 17:58:58 +0300 (EEST)
Received: by mail-wm1-f49.google.com with SMTP id
5b1f17b1804b1-43d07ca6a80so20113365e9.1
for <ffmpeg-devel@ffmpeg.org>; Mon, 21 Apr 2025 07:58:58 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=jkqxz-net.20230601.gappssmtp.com; s=20230601; t=1745247537; x=1745852337;
darn=ffmpeg.org;
h=content-transfer-encoding:in-reply-to:from:references:to
:content-language:subject:user-agent:mime-version:date:message-id
:from:to:cc:subject:date:message-id:reply-to;
bh=drj0cLoUpZ3AXzt8MM+6qkLBR8Y4NwqysfpipzWxgTs=;
b=XcQCIOnERh04r/zXqOJH0+jogDytI0lCkIRWOeaEHTkSqiDhArniL5Jw1SDxvJR3BF
rlSoz6vQ9JxgIEkS1x0cSQV8Mk34Ww9fzxusoZkV0Gm4BOjo8eC+3U/rZCadLRegY/RV
nkl6+h9UQIITVU9kaHzZYGe46/GrmuyuLhf7Hid+REiO65zLg2BckBAOvJ/lD3vjo/Al
r/0iSynFq+NWYvg4t45yd7Yjl7OWa06hjc6t1uSDLtX9OCdZ6yeOAS6W6B1PcRjJShgC
r7UCOhLj4bAYLwRop+eVnljy9iuXEBpCtP98Ke714muz4NPRzv9k95N4VTVidbDkuD+A
3Ndg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1745247537; x=1745852337;
h=content-transfer-encoding:in-reply-to:from:references:to
:content-language:subject:user-agent:mime-version:date:message-id
:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;
bh=drj0cLoUpZ3AXzt8MM+6qkLBR8Y4NwqysfpipzWxgTs=;
b=KfpNRCyIXYeZlIWZSo+KWRl5BjAaYediNPPfzH3YFYARhB7ogfbw7qXq73GHOy5A2d
SgygJZZxpV3oQDWNkC0wnsK7bFO4YknA68h84laLo9HGn2iEWOBiFlRzRC/DhOdezCSm
r0JFHWxssKV5SIjV5zYWTQBnwRjvFTCeLVxpeEhlz7eDi0mJ1eHFknk1ko2otMPOFxsb
GhOy1btx49EE/JxGJ7ApW58o+q9GHxtSEalEfme1DLO6kODduWbixvvW2anrcqiDtEDt
p0MuB1rOq0mK9HzMFXbLaL3V6olY8oYofx4zJSOdJhEurRfmcZE7wO1ONh4tKaXure8O
j9aA==
X-Gm-Message-State: AOJu0Yx5++PWnFCVDYlNifvypbvQNujTev3pUe+NnSZ1ZKSsquqWAUzp
QaEmMXGg6zbb7NVGTwnAMCmeFgcI0UQueH2G6bMKRr3srFlyDmnz1ymfSumU0TCjUt4Y+B+Y70d
c
X-Gm-Gg: ASbGncubyOtomHDyTS0hiCkgT4l9tVlK6nFd57PXn2aa+oAYPylsjGBwQ+SW8Edodyv
/DKcWWQoOstSYDUwB1vq1a6UcpVCA4QDrlIFqEN0+MjMYuNO2I/mD+cGMuVVJon5iTazp8u9d2r
oidUFArlNzTCJqYynzqBh8A9LgpXF9oaSxg1tu3d/EQldoeorqRIXV0dVH/6NBcrftpwJjLV1jl
zyVAP76fJTKLLbkufnX1qgVKE/CV32ILtHMHYj+/QI4aMXPdIecC5OtcnKQKJb3JHBxsFTVoPAM
9TKnPWef8U9PCGKy5Gsw3dSRJ/oCqBav1jUhBdTlumvaK/1ZGoJX8th4WQwF2GncUhr6CCdOmA7
dmEh3aIABt9SiWi2+7mvr2Shi
X-Google-Smtp-Source: AGHT+IHH5kji97mSJYoAxTA/KXHzkG3VUnhg91EAaKPsWTLWeitKEJzwGnwrkuMogoYJkL3PFCJJbA==
X-Received: by 2002:a05:600c:384f:b0:440:59eb:bfc with SMTP id
5b1f17b1804b1-4406abf94bamr92829395e9.23.1745247537353;
Mon, 21 Apr 2025 07:58:57 -0700 (PDT)
Received: from [192.168.0.15]
(cpc92320-cmbg19-2-0-cust719.5-4.cable.virginm.net. [82.13.66.208])
by smtp.gmail.com with ESMTPSA id
5b1f17b1804b1-4406d5a9e1bsm143007375e9.8.2025.04.21.07.58.57
for <ffmpeg-devel@ffmpeg.org>
(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);
Mon, 21 Apr 2025 07:58:57 -0700 (PDT)
Message-ID: <8692bb91-8844-4773-9d1c-08240fae74f5@jkqxz.net>
Date: Mon, 21 Apr 2025 15:59:00 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-US
To: ffmpeg-devel@ffmpeg.org
References: <20250419190712.1265201-1-sw@jkqxz.net>
<20250419190712.1265201-3-sw@jkqxz.net> <20250421005448.GL4991@pb2>
From: Mark Thompson <sw@jkqxz.net>
In-Reply-To: <20250421005448.GL4991@pb2>
Subject: Re: [FFmpeg-devel] [PATCH 2/6] lavf: APV demuxer
X-BeenThere: ffmpeg-devel@ffmpeg.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: FFmpeg development discussions and patches <ffmpeg-devel.ffmpeg.org>
List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe>
List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel>
List-Post: <mailto:ffmpeg-devel@ffmpeg.org>
List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help>
List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe>
Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/8692bb91-8844-4773-9d1c-08240fae74f5@jkqxz.net/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
On 21/04/2025 01:54, Michael Niedermayer wrote:
> 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
I will reorder the cbs patch before this one.
>> +#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
Ok.
>> +
>> + 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
Sure. We can change it to uint32_t later if there is a decision to use APVDecoderConfigurationRecord directly.
> [...]
>> +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) {
Here ^
>> + // 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
See check above.
Thanks,
- Mark
_______________________________________________
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".