From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 6F1EC45AE1 for ; Wed, 17 May 2023 00:40:46 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8EC7A68BFDB; Wed, 17 May 2023 03:40:43 +0300 (EEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 963F068BE7F for ; Wed, 17 May 2023 03:40:36 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1684284041; x=1715820041; h=from:to:subject:date:message-id:references:in-reply-to: content-transfer-encoding:mime-version; bh=/SX7Q6c4Pi3/Q+jeCrkZy7pU/XpsQUqz2IBI9WWUBiQ=; b=DVVh6z5YbB4ev9N7AVbDZ5lB6ChPBIZqdgh7M3T4LbyOgqlAyDp6OI2I WmgUnmt+NPu1dArmOA1rF5cdlm7TbkqdWiVtzvh/+nQi1GV8sHPWbanpX fQRYfJlUJWcxrlckv+6UJuF0IfRS984r6EnC96znAZyrCpztp1GwiWhpI fKogIIQjj+5RcQQueZBeZnxrBkAUbA+bvqRUBkwV8UbXUytBcqZUD/gr4 /FH+4yjPnuPaCGIfRbXszKNQSYagyCbuINsqKYQD0SmR9zI3aOdquvkBW a9FgVGlLi2MpRCbe44V2ZX4XjFejEWsD/TVYkvGoRuEBRybmOXf34i0Ph w==; X-IronPort-AV: E=McAfee;i="6600,9927,10712"; a="353915318" X-IronPort-AV: E=Sophos;i="5.99,280,1677571200"; d="scan'208";a="353915318" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 May 2023 17:40:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10712"; a="734494499" X-IronPort-AV: E=Sophos;i="5.99,280,1677571200"; d="scan'208";a="734494499" Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by orsmga001.jf.intel.com with ESMTP; 16 May 2023 17:40:33 -0700 Received: from orsmsx612.amr.corp.intel.com (10.22.229.25) by ORSMSX603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Tue, 16 May 2023 17:40:33 -0700 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) by orsmsx612.amr.corp.intel.com (10.22.229.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23 via Frontend Transport; Tue, 16 May 2023 17:40:33 -0700 Received: from NAM02-SN1-obe.outbound.protection.outlook.com (104.47.57.47) by edgegateway.intel.com (134.134.137.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.23; Tue, 16 May 2023 17:40:33 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=M09lTKAnaH+PRl/XaAK4lKt4cr7WWrf1TS9RtcFGZZKuaNqiSfnjPH47+Yx0OggmdpFwft6fu2gkWzwV9ItGsIsbNpCpqrle7Rr0/pke/AAxw9apxjVvFsJjDoNXOKNMJA3fE8jSLseYMsYQvYzxohYqxRsMX6skC6ORrAA1laF1onBL0J+uajS8KyjTNfGnU89JhEJ8npNVdDEAqpWSoYHni7P5rMydDmNuEo8fKMcLAuwGwaw7YNl5AgvgZhSFlYHW/Xp/5pJAfYdNECpHkyfb0rjEB94ahLjExuwdj7Dx0SqMyzW6xMj8OKPzCODfkGK33sw/mCO5TD0SxtCHvg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=igKoAytK7V9RiLIx3yEHS5GhkaWCkoyFLjI7Qqaz00w=; b=jeI7ws3krXy0k5rEzNcXreypTMIgbAe2WRvzh6srtCjNX2eAWXcCFE0WYRig5LhtbJwvWOQin3242zSNev1DrIuWTX75fFojqeWACVpmn68iLpaZrlOK/CmDSfb6Kx9m/SW/ql2DlLrVR5gDe+VElzaoC3u8xRblmCknZH0YmC5JV5Bw2cyNuqKPlOh/cJmOd+0Q/Ev+FBOGAGlVmy7HLGSIwC0tPzQm2D7oPmMMx/NcN/IGgASI0MiMajLPEdACpg2MA8O4miG/pIY5JxpZSmBdt8Qr8ZrImUNKgOckZtXv960WGk37orF/A556zGgUokXBd/5218WPFE5CbVWGBQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from DM6PR11MB2681.namprd11.prod.outlook.com (2603:10b6:5:bd::33) by SA2PR11MB5161.namprd11.prod.outlook.com (2603:10b6:806:fa::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6387.33; Wed, 17 May 2023 00:40:30 +0000 Received: from DM6PR11MB2681.namprd11.prod.outlook.com ([fe80::c994:98fe:fd3a:ffcb]) by DM6PR11MB2681.namprd11.prod.outlook.com ([fe80::c994:98fe:fd3a:ffcb%5]) with mapi id 15.20.6411.017; Wed, 17 May 2023 00:40:30 +0000 From: "Dai, Jianhui J" To: FFmpeg development discussions and patches Thread-Topic: [FFmpeg-devel] [PATCH V1] avcodec/cbs_vp8: Add support for VP8 codec bitstream READ methods Thread-Index: AdmG0tDkkKnQPJM6TRS9lBYKUl3QKgAMcNMAAFPvy5A= Date: Wed, 17 May 2023 00:40:29 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: DM6PR11MB2681:EE_|SA2PR11MB5161:EE_ x-ms-office365-filtering-correlation-id: 1a6f96ce-a19d-437e-f34a-08db566f50ab x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: YOigl8oHdROCjIJ627TGeR2wa4Vh80w7JXwnNxFdJgKzOrbv7zDvbnalbKtkojp2DMmUVqbg6owomrYy0j+Tkwkb03H8fUkhn3WFRrnKp/ROH/YuNIw3ZDIxd/efEHncpyafw9IS2yrQMzVphteVzO+3cyiO+HD/QxE7ZsBScNsy56YURdnWkLlxAGDmJiznf4mw4QP34dqzaz/E24H5rGZ3gikqFzZTowri3u4JCNG/PbzKi+75NPZXE8brKTviyR9bCnlwtgcJDmhBJ/LvL4hx2752aorGw22RQx/GRpmerYIFn5z+19+vFN10wMgeg/J/kRv+UD3HuddHIGefWkDZ9VaAccgBP+8OtqMyofp0ttOFBV8LsFGbbvNCcZsGwkE1/9Q/7yJVZXtGZdnKl8oHjxuXftQpRZ/NmihSon6E4LVZOF9USbI8QPzPn9HFFxVvoV6lymRdk5psYiKONj8dGinY24eAgQVUDr5PsYAfCqtMmYrh8dqghG4USO1ljHT7JxPy44i4XBJtqeesKBG0z8KlquGk1ZXHokCUB3oESIPG2SRYRs0fC22YOkL9gtFTxRspYySGC7ruyA1fPExjyl9zt1c3BaieWUjsRvNFd4NR7muR0i9GPLs22QLl x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR11MB2681.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230028)(39860400002)(376002)(136003)(346002)(366004)(396003)(451199021)(83380400001)(53546011)(966005)(478600001)(71200400001)(33656002)(7696005)(19627235002)(6506007)(26005)(186003)(9686003)(2906002)(30864003)(38070700005)(38100700002)(122000001)(64756008)(66446008)(6916009)(66946007)(76116006)(66476007)(66556008)(82960400001)(41300700001)(5660300002)(316002)(8676002)(52536014)(8936002)(55016003)(86362001)(66899021)(2004002)(559001)(579004); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?bptgnBKqHFmIcMlnmzQ2nsR3jXUlmU5eYmaWwbRFXosYLJLNHFF+uj/b1PhZ?= =?us-ascii?Q?vOgoZU12rJ3mvLd0J1Pl32C7b36Gf9eDY+SvKrORQ/0UEcjysmLFJHm8MKef?= =?us-ascii?Q?3LlHNaZleVxih9fFry+svkjPEMpS/oyLOBit+rQG9ztyURKIrK7SPytH1iPt?= =?us-ascii?Q?A6UN7sY7Q3MzHxW1gw6r80//pRGlAOr5ysCe+NFEa0GlWVeiOPtB6smjGlrk?= =?us-ascii?Q?pp3Y6Rtkr6/Dd988g+71TVk37KZOU28Zg6RSu8z/DvmNHhuqvf1t9Ei9NSTs?= =?us-ascii?Q?EU1Ih9EZ8FTLq6e4FbvprDcGSBIfiKT+Q9wuM2fbUKBBpO/cplLWnmyDJikG?= =?us-ascii?Q?RsabQNSdh4yeJi3a4xVLClM42ua1EMsfTfrzBPkYJRjEpwygosHlSr2Mx8u9?= =?us-ascii?Q?zY946MXpKtMIetYMKkOgFmtLyvd74DZjvTRytMC0S1dVrUNlume1Hdddlc6i?= =?us-ascii?Q?QK2iO8xTtCAIj5rKaFHxBEhMlX/Wd3tGV1jJpcKN+t8QS3VZIgZeqqek6opL?= =?us-ascii?Q?rtD4XtXiuE46bpLdNG+kQNd4IwNxrQza/TYuwZWeQ2I/zZCNP18+HINgh6gU?= =?us-ascii?Q?zlC38pa7Yq/sR9hsHiNICvblXZXx4datj+c/4pJ3n+zw2l0lnDwl777CpFzm?= =?us-ascii?Q?Mu3j/qw3ymo77Oz8DG0LlqUK4RMuqAzupHRxWR4JjzUztQrPkFOqDO3odQOs?= =?us-ascii?Q?Axpd6bfwiOou/ROfEmFCw34glQQrD//gLzg0UIeL3KvvP+0vsKqzfxKhz20y?= =?us-ascii?Q?Zrcm41bALP5V+Y81DXAYuezHlXAhdeMfmICtupCWbI8VsvPT5TCqduV2LDt7?= =?us-ascii?Q?iaK5bS+kHpdLMnNsKbfbE2GAzg+cOqCoThq8TSw0+6dFIJJ5VUiIUNowkuD9?= =?us-ascii?Q?P1W+ezCmS0hJ/WNktZ/ADyeqZO/XT2bMFeVUJEtF5sp9I8MVEt3QkoRnfrhq?= =?us-ascii?Q?/nMHGxTocIPSaYb2SiiJG1Pq6XUEp9ddDMuunbK3ljiRd9IfMAd99wpbTzZ5?= =?us-ascii?Q?m992CbYXZeaN0zvlAxReFB/UjvG7cibcLPxP3faG4kKhP8JDnraH3de3Je6p?= =?us-ascii?Q?+LP9ce0QjXeDtI/TVEeygRyePNYB5+MXsSHOFyDzqiXJH5cG7W3oy/ISj0l+?= =?us-ascii?Q?4Q8+8kC2HaDNP9lcqj6hmB5B8x40nDTNpPeQ4s7D2J08mw0yOCoEi7vI8XBK?= =?us-ascii?Q?By5EFdX2kJ72LWEgeAdI01PlZi05nSBRd42mqtR2vbNm2sC6fd2+JKNxwIqz?= =?us-ascii?Q?e4Po2ULufJ5bIUTUjoC1mxDlCFzPY6NGokbwOIirZqSC2ybAwZUhksLuIqEz?= =?us-ascii?Q?362AeLCunrpFubCzBgeBKcYeJrAHg1OTD/vR64HoBtsQxiNavwYR4qz7jHFj?= =?us-ascii?Q?wFSsn2J51QFQRLT4QS9lWpmkWdtjd2MQYPJ9ElOGVtQP+bdijZ1HF0xFaLEc?= =?us-ascii?Q?7/rpqnn+TBxeY3QqzRd6yx5E9gJRfTKpw6k9DRxqET3g+MSc1gG+Cqe1M2+t?= =?us-ascii?Q?n/qMdVs3/BPUzxJAKr1YRJA2ycgROJy0GspkxgWCgNRJdsEamSa2dvEqoULG?= =?us-ascii?Q?i09I8vNCMETAQn2B1SzD+2UP9OJGX/wjwzlNDqWi?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB2681.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 1a6f96ce-a19d-437e-f34a-08db566f50ab X-MS-Exchange-CrossTenant-originalarrivaltime: 17 May 2023 00:40:29.7560 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: rTEPVS+qUHbVMRnB5myCBOQOi7SIJ1XIfo1u8Ay0bN3dGIMvSKdj73lTr33JB29+LWAjZ1M/+++PrIrxqhZ6lA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5161 X-OriginatorOrg: intel.com Subject: Re: [FFmpeg-devel] [PATCH V1] avcodec/cbs_vp8: Add support for VP8 codec bitstream READ methods X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: > -----Original Message----- > From: ffmpeg-devel On Behalf Of > Andreas Rheinhardt > Sent: Monday, May 15, 2023 4:10 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH V1] avcodec/cbs_vp8: Add support for VP8 > codec bitstream READ methods > > Dai, Jianhui J: > > This commit adds VP8 into cbs supported codec list, and enables the > > `trace_headers` bitstream filters to support VP8, besides existing > > AV1, H.264, H.265 and VP9. It can be useful to debug VP8 stream issues. > > > > Only the READ methods `read_unit` and `split_fragment` are > > implemented, the WRITE methods `write_unit` and `assemble_fragment` > > return `AVERROR_PATCHWELCOME` error code. It is because the CBS VP8 > > WRITE is unlikely used by any applications at the moment. The WRITE > > methods can be added later if there are real requirments. > > > > TESTS: ffmpeg -i fate-suite/vp8/frame_size_change.webm -vcodec copy > > -bsf:v trace_headers -f null - > > > > Signed-off-by: Jianhui Dai > > --- > > configure | 4 +- > > libavcodec/Makefile | 1 + > > libavcodec/cbs.c | 6 + > > libavcodec/cbs_internal.h | 1 + > > libavcodec/cbs_vp8.c | 259 +++++++++++++++++++++++ > > libavcodec/cbs_vp8.h | 116 +++++++++++ > > libavcodec/cbs_vp8_syntax_template.c | 294 > > +++++++++++++++++++++++++++ > > 7 files changed, 680 insertions(+), 1 deletion(-) create mode 100644 > > libavcodec/cbs_vp8.c create mode 100644 libavcodec/cbs_vp8.h create > > mode 100644 libavcodec/cbs_vp8_syntax_template.c > > > > diff --git a/configure b/configure > > index bb7be67676..6ba7759e34 100755 > > --- a/configure > > +++ b/configure > > @@ -2433,6 +2433,7 @@ CONFIG_EXTRA=" > > cbs_jpeg > > cbs_mpeg2 > > cbs_vp9 > > + cbs_vp8 > > vp8 should be before vp9 in all these lists. Thank for your review. ACK. > > > deflate_wrapper > > dirac_parse > > dnn > > @@ -2714,6 +2715,7 @@ cbs_h265_select="cbs" > > cbs_jpeg_select="cbs" > > cbs_mpeg2_select="cbs" > > cbs_vp9_select="cbs" > > +cbs_vp8_select="cbs" > > dct_select="rdft" > > deflate_wrapper_deps="zlib" > > dirac_parse_select="golomb" > > @@ -3284,7 +3286,7 @@ h264_redundant_pps_bsf_select="cbs_h264" > > hevc_metadata_bsf_select="cbs_h265" > > mjpeg2jpeg_bsf_select="jpegtables" > > mpeg2_metadata_bsf_select="cbs_mpeg2" > > -trace_headers_bsf_select="cbs" > > +trace_headers_bsf_select="cbs cbs_vp8" > > vp9_metadata_bsf_select="cbs_vp9" > > > > # external libraries > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile index > > 3cf4444b7e..a4e5d7d7a5 100644 > > --- a/libavcodec/Makefile > > +++ b/libavcodec/Makefile > > @@ -79,6 +79,7 @@ OBJS-$(CONFIG_CBS_H265) += cbs_h2645.o > cbs_sei.o h2645_parse.o > > OBJS-$(CONFIG_CBS_JPEG) += cbs_jpeg.o > > OBJS-$(CONFIG_CBS_MPEG2) += cbs_mpeg2.o > > OBJS-$(CONFIG_CBS_VP9) += cbs_vp9.o > > +OBJS-$(CONFIG_CBS_VP8) += cbs_vp8.o vpx_rac.o > > OBJS-$(CONFIG_CRYSTALHD) += crystalhd.o > > OBJS-$(CONFIG_DCT) += dct.o dct32_fixed.o dct32_float.o > > OBJS-$(CONFIG_DEFLATE_WRAPPER) += zlib_wrapper.o > > diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c index > > 504197e06d..f4ff05181f 100644 > > --- a/libavcodec/cbs.c > > +++ b/libavcodec/cbs.c > > @@ -49,6 +49,9 @@ static const CodedBitstreamType *const > > cbs_type_table[] = { #if CONFIG_CBS_VP9 > > &ff_cbs_type_vp9, > > #endif > > +#if CONFIG_CBS_VP8 > > + &ff_cbs_type_vp8, > > +#endif > > }; > > > > const enum AVCodecID ff_cbs_all_codec_ids[] = { @@ -69,6 +72,9 @@ > > const enum AVCodecID ff_cbs_all_codec_ids[] = { #endif #if > > CONFIG_CBS_VP9 > > AV_CODEC_ID_VP9, > > +#endif > > +#if CONFIG_CBS_VP8 > > + AV_CODEC_ID_VP8, > > #endif > > AV_CODEC_ID_NONE > > }; > > diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h > > index e585c77934..9ea6393a9f 100644 > > --- a/libavcodec/cbs_internal.h > > +++ b/libavcodec/cbs_internal.h > > @@ -248,6 +248,7 @@ extern const CodedBitstreamType ff_cbs_type_h265; > > extern const CodedBitstreamType ff_cbs_type_jpeg; extern const > > CodedBitstreamType ff_cbs_type_mpeg2; extern const CodedBitstreamType > > ff_cbs_type_vp9; > > +extern const CodedBitstreamType ff_cbs_type_vp8; > > > > > > #endif /* AVCODEC_CBS_INTERNAL_H */ > > diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c new file mode > > 100644 index 0000000000..ee5c5cbceb > > --- /dev/null > > +++ b/libavcodec/cbs_vp8.c > > @@ -0,0 +1,259 @@ > > +/* > > + * 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/avassert.h" > > + > > +#include "cbs.h" > > +#include "cbs_internal.h" > > +#include "cbs_vp8.h" > > + > > +#include "vp8data.h" > > + > > +static av_unused int > > +cbs_vp8_trace_syntax_element(CodedBitstreamContext *ctx, > > Why is this marked as av_unused? ACK > > > + int pos, const char *name, > > + const int *subscripts, > > + int32_t value, int > > +width) { > > + if (ctx->trace_enable) { > > + char bits[33] = "-"; > > + > > + av_assert0(width >= 0 && width <= 24); > > + if (width > 0) { > > + int i; > > + for (i = 0; i < width; i++) > > + bits[i] = value >> (width - i - 1) & 1 ? '1' : '0'; > > + bits[i] = 0; > > + } > > + > > + ff_cbs_trace_syntax_element(ctx, pos, name, subscripts, bits, value); > > + } > > + > > + return 0; > > +} > > + > > +static int cbs_vp8_read(CodedBitstreamContext *ctx, GetBitContext *gbc, > > + int width, int32_t *write_to) { > > + uint32_t value; > > + > > + av_assert0(width > 0 && width <= 24); > > + > > + if (get_bits_left(gbc) < width + 1) { > > Why the +1? ACK > > > + av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid value: bitstream > ended.\n"); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + value = get_bits_le(gbc, width); > > You are reading from a bitstream reader that has not been opened as little- > endian. This will work, but it is not how it is supposed to be. ACK > > > + *write_to = value; > > + return 0; > > +} > > + > > +static av_unused int cbs_vp8_rac_get(VPXRangeCoder *c, int bits) { > > + int value = 0; > > + > > + while (bits--) > > + value = (value << 1) | vpx_rac_get_prob(c, 128); > > + > > + return value; > > +} > > + > > +static int cbs_vp8_rac_get_bits_count(GetBitContext *gbc, > > +VPXRangeCoder *c) { > > + int bits_count = (c->buffer - gbc->buffer) * 8; > > + bits_count += c->bits; > > Your mixing of VPXRangeCoder and GetBitContext is incredibly hacky; if I see > this correctly, then you are not even advancing the GetBitContext position for all > the rac stuff that you are reading (meaning that the byte-alignment code in > trailing_bits() is nonsense). Right. VP8 bitstream comprise 3 parts: Frame Tag (uncompressed header), Frame Header (Boolean Entropy Encoder) and MBs. The `Frame Tag` is parsed by GetBitContext, ` Frame Header` is parsed by VPXRangeCoder. I should check trailing_bits() after FrameHeader. Will fix it > Furthermore, the rac functions do not report when they run out of input, as is > common for CBS. You should probably write CBS versions of them that closely > follow the spec (read: that need not be optimized for speed). I would like the cbs_vp8 to be lightweight, that reuse existing VPXRangeCoder. I will check and fix it. > > > + return bits_count; > > +} > > + > > +#define HEADER(name) \ > > + do { \ > > + ff_cbs_trace_header(ctx, name); \ > > + } while (0) > > + > > +#define CHECK(call) \ > > + do { \ > > + err = (call); \ > > + if (err < 0) \ > > + return err; \ > > + } while (0) > > + > > +#define FUNC_NAME(rw, codec, name) cbs_##codec##_##rw##_##name > > +#define FUNC_VP8(rw, name) FUNC_NAME(rw, vp8, name) #define > > +FUNC(name) FUNC_VP8(READWRITE, name) > > + > > +#define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs, > > +__VA_ARGS__ }) : NULL) > > + > > +#define vp8_f(width, name) vp8_xf(width, name, current->name) > > + > > +#define vp8_rac_unsigned(width, name) \ > > + do { \ > > + uint32_t value; \ > > + int pos = cbs_vp8_rac_get_bits_count(rw, c); \ > > + value = cbs_vp8_rac_get(c, width); \ > > + cbs_vp8_trace_syntax_element(ctx, pos, #name, 0, value, 0); \ > > IMO it is better to have a function cbs_vp8_rac_read_unsigned() for this; > vp8_rac_unsigned and vp8_rac_unsigned_subs would then just be wrappers > around this function like it is done for xs() in cbs_vp9. ACK > > > + current->name = value; \ > > + } while (0) > > + > > +#define vp8_rac_unsigned_subs(width, name, subs, ...) \ > > + do { \ > > + uint32_t value; \ > > + int pos = cbs_vp8_rac_get_bits_count(rw, c); \ > > + value = cbs_vp8_rac_get(c, width); \ > > + cbs_vp8_trace_syntax_element(ctx, pos, #name, \ > > + SUBSCRIPTS(subs, __VA_ARGS__), value, 0); \ > > + current->name = value; \ > > + } while (0) > > + > > +#define vp8_rac_signed(width, name) \ > > + do { \ > > + uint32_t value; \ > > Why is this not signed? Same below. ACK > > > + int pos = cbs_vp8_rac_get_bits_count(rw, c); \ > > + value = cbs_vp8_rac_get(c, width); \ > > + if (cbs_vp8_rac_get(c, 1)) \ > > + value = -value; \ > > + cbs_vp8_trace_syntax_element(ctx, pos, #name, 0, value, 0); \ > > Same as above for vp8_rac_unsigned. ACK > > > + current->name = value; \ > > + } while (0) > > + > > +#define vp8_rac_signed_subs(width, name, subs, ...) \ > > + do { \ > > + uint32_t value; \ > > + int pos = cbs_vp8_rac_get_bits_count(rw, c); \ > > + value = cbs_vp8_rac_get(c, width); \ > > + if (cbs_vp8_rac_get(c, 1)) \ > > + value = -value; \ > > + cbs_vp8_trace_syntax_element(ctx, pos, #name, \ > > + SUBSCRIPTS(subs, __VA_ARGS__), value, 0); \ > > + current->name = value; \ > > + } while (0) > > + > > +#define READ > > +#define READWRITE read > > +#define RWContext GetBitContext > > + > > +#define vp8_xf(width, name, var) \ > > + do { \ > > + uint32_t value; \ > > + int pos = get_bits_count(rw); \ > > + CHECK(cbs_vp8_read(ctx, rw, width, &value)); \ > > + var = value; \ > > + cbs_vp8_trace_syntax_element(ctx, pos, #name, 0, value, width); \ > > + } while (0) > > + > > +#define fixed(width, name, value) \ > > + do { \ > > + av_unused uint32_t fixed_value; \ > > + CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, 0, &fixed_value, \ > > + value, value)); \ > > + } while (0) > > + > > +#define byte_alignment(rw) (get_bits_count(rw) % 8) > > + > > +#include "cbs_vp8_syntax_template.c" > > + > > +static int cbs_vp8_split_fragment(CodedBitstreamContext *ctx, > > + CodedBitstreamFragment *frag, int > > +header) { > > + int err; > > + > > + if (frag->data_size == 0) > > + return AVERROR_INVALIDDATA; > > + > > + err = ff_cbs_append_unit_data(frag, 0, frag->data, frag->data_size, > > + frag->data_ref); > > + if (err < 0) > > + return err; > > + > > + return 0; > > +} > > + > > +static int cbs_vp8_read_unit(CodedBitstreamContext *ctx, > > + CodedBitstreamUnit *unit) { > > + VP8RawFrame *frame; > > + GetBitContext gbc; > > + int err, pos; > > + > > + err = init_get_bits(&gbc, unit->data, 8 * unit->data_size); > > + if (err < 0) > > + return err; > > + > > + err = ff_cbs_alloc_unit_content(ctx, unit); > > + if (err < 0) > > + return err; > > + frame = unit->content; > > + > > + err = cbs_vp8_read_frame(ctx, &gbc, frame); > > + if (err < 0) > > + return err; > > + > > + pos = get_bits_count(&gbc); > > + av_assert0(pos % 8 == 0); > > + pos /= 8; > > + av_assert0(pos <= unit->data_size); > > + > > + if (pos == unit->data_size) > > + return AVERROR_INVALIDDATA; > > + > > + frame->data_ref = av_buffer_ref(unit->data_ref); > > + if (!frame->data_ref) > > + return AVERROR(ENOMEM); > > + > > + frame->data = unit->data + pos; > > + frame->data_size = unit->data_size - pos; > > + > > + return 0; > > +} > > + > > +static int cbs_vp8_write_unit(CodedBitstreamContext *ctx, > > + CodedBitstreamUnit *unit, PutBitContext > > +*pbc) { > > + return AVERROR_PATCHWELCOME; > > +} > > + > > +static int cbs_vp8_assemble_fragment(CodedBitstreamContext *ctx, > > + CodedBitstreamFragment *frag) { > > + return AVERROR_PATCHWELCOME; > > +} > > + > > +static void cbs_vp8_flush(CodedBitstreamContext *ctx) { > > + return; > > +} > > No need for a useless flush. Just leave the function empty. > (One could also do the same for the write_unit and assemble_fragment > function: Nothing should call these functions at all.) ACK > > > + > > +static const CodedBitstreamUnitTypeDescriptor cbs_vp8_unit_types[] = { > > + CBS_UNIT_TYPE_INTERNAL_REF(0, VP8RawFrame, data), > > + CBS_UNIT_TYPE_END_OF_LIST > > +}; > > + > > +const CodedBitstreamType ff_cbs_type_vp8 = { > > + .codec_id = AV_CODEC_ID_VP8, > > + > > + .priv_data_size = sizeof(CodedBitstreamVP8Context), > > + > > + .unit_types = cbs_vp8_unit_types, > > + > > + .split_fragment = &cbs_vp8_split_fragment, > > + .read_unit = &cbs_vp8_read_unit, > > + .write_unit = &cbs_vp8_write_unit, > > + > > + .flush = &cbs_vp8_flush, > > + > > + .assemble_fragment = &cbs_vp8_assemble_fragment, }; > > diff --git a/libavcodec/cbs_vp8.h b/libavcodec/cbs_vp8.h new file mode > > 100644 index 0000000000..176d95853b > > --- /dev/null > > +++ b/libavcodec/cbs_vp8.h > > @@ -0,0 +1,116 @@ > > +/* > > + * 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 */ > > + > > +#ifndef AVCODEC_CBS_VP8_H > > +#define AVCODEC_CBS_VP8_H > > + > > +#include > > +#include > > + > > +#include "cbs.h" > > +#include "vpx_rac.h" > > + > > +enum { > > + VP8_START_CODE_0 = 0x9D, > > + VP8_START_CODE_1 = 0x01, > > + VP8_START_CODE_2 = 0x2A, > > +}; > > + > > +enum { > > + VP8_KEY_FRAME = 0, > > + VP8_NON_KEY_FRAME = 1, > > +}; > > + > > +typedef struct VP8RawFrameHeader { > > + // frame tag > > + uint8_t frame_type; > > + uint8_t profile; > > + uint8_t show_frame; > > + uint32_t first_partition_length_in_bytes; > > + > > + uint32_t start_code_0; > > + uint32_t start_code_1; > > + uint32_t start_code_2; > > + > > + uint16_t width; > > + uint16_t horizontal_scale; > > + uint16_t height; > > + uint16_t vertical_scale; > > + > > + // frame header > > + uint8_t color_space; > > + uint8_t clamping_type; > > + > > + // segmentation > > + uint8_t segmentation_enabled; > > + uint8_t update_segment_map; > > + uint8_t update_segment_feature_data; > > + uint8_t segment_feature_mode; > > + int8_t segment_update_qp[4]; > > + int8_t segment_update_loop_filter_level[4]; > > + uint8_t segment_update_probs[3]; > > + > > + // loop filter > > + uint8_t loop_filter_type; > > + uint8_t loop_filter_level; > > + uint8_t loop_filter_sharpness; > > + uint8_t mode_ref_lf_delta_enabled; > > + int8_t ref_lf_deltas[4]; > > + int8_t mode_lf_deltas[4]; > > + > > + uint8_t log2_token_partitions; > > + > > + // qp > > + uint8_t base_qindex; > > + int8_t y1dc_delta_q; > > + int8_t y2dc_delta_q; > > + int8_t y2ac_delta_q; > > + int8_t uvdc_delta_q; > > + int8_t uvac_delta_q; > > + > > + // ref > > + uint8_t refresh_golden_frame; > > + uint8_t refresh_alternate_frame; > > + uint8_t ref_frame_sign_bias_golden; > > + uint8_t ref_frame_sign_bias_alternate; > > + uint8_t refresh_last_frame; > > + > > + // token probs > > + uint8_t refresh_entropy_probs; > > + > > + uint8_t mb_no_skip_coeff; > > + uint8_t prob_skip_false; > > + > > + uint8_t prob_intra; > > + uint8_t prob_last; > > + uint8_t prob_golden; > > +} VP8RawFrameHeader; > > + > > +typedef struct VP8RawFrame { > > + VP8RawFrameHeader header; > > + > > + uint8_t *data; > > + AVBufferRef *data_ref; > > + size_t data_size; > > +} VP8RawFrame; > > + > > +typedef struct CodedBitstreamVP8Context { > > + VPXRangeCoder c; > > +} CodedBitstreamVP8Context; > > + > > +#endif /* AVCODEC_CBS_VP8_H */ > > diff --git a/libavcodec/cbs_vp8_syntax_template.c > > b/libavcodec/cbs_vp8_syntax_template.c > > new file mode 100644 > > index 0000000000..24f4075dab > > --- /dev/null > > +++ b/libavcodec/cbs_vp8_syntax_template.c > > @@ -0,0 +1,294 @@ > > +/* > > + * 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 */ > > + > > +static int FUNC(update_segmentation)(CodedBitstreamContext *ctx, > RWContext *rw, > > + VPXRangeCoder *c, > > + VP8RawFrameHeader *current) { > > +#ifdef READ > > + int i; > > We allow (and prefer) C99 for loops with variable declarations. ACK > > > + > > + vp8_rac_unsigned(1, update_segment_map); > > + vp8_rac_unsigned(1, update_segment_feature_data); > > + > > + if (current->update_segment_feature_data) { > > + vp8_rac_unsigned(1, segment_feature_mode); > > + // quantizer > > + for (i = 0; i < 4; i++) > > + if (cbs_vp8_rac_get(c, 1)) > > + vp8_rac_signed_subs(7, segment_update_qp[i], 1, i); > > + // loop filter > > + for (i = 0; i < 4; i++) { > > + if (cbs_vp8_rac_get(c, 1)) { > > + vp8_rac_signed_subs(6, segment_update_loop_filter_level[i], 1, > > + i); > > + } > > + } > > + } > > + > > + if (current->update_segment_map) { > > + for (i = 0; i < 3; i++) > > + if (cbs_vp8_rac_get(c, 1)) > > + vp8_rac_unsigned_subs(8, segment_update_probs[i], 1, i); > > + } > > + > > + return 0; > > +#else > > + return AVERROR_PATCHWELCOME; > > +#endif > > IMO there is no need for these #else branches given that this file is not included > for anything but reading. The intention is that possible we want the cbs_vp8 WRITE methods in near future. If the WRITE methods are unlikely needed, I will remove it. > > > +} > > + > > +static int FUNC(mode_ref_lf_deltas)(CodedBitstreamContext *ctx, > > + RWContext *rw, VPXRangeCoder *c, > > + VP8RawFrameHeader > > +*current) { #ifdef READ > > + vp8_rac_unsigned(1, mode_ref_lf_delta_enabled); > > + if (current->mode_ref_lf_delta_enabled) { > > + if (cbs_vp8_rac_get(c, 1)) { > > + int i; > > + // ref_lf_deltas > > + for (i = 0; i < 4; i++) > > + if (cbs_vp8_rac_get(c, 1)) > > + vp8_rac_signed_subs(6, ref_lf_deltas[i], 1, i); > > + // mode_lf_deltas > > + for (i = 0; i < 4; i++) > > + if (cbs_vp8_rac_get(c, 1)) > > + vp8_rac_signed_subs(6, mode_lf_deltas[i], 1, i); > > + } > > + } > > + > > + return 0; > > +#else > > + return AVERROR_PATCHWELCOME; > > +#endif > > +} > > + > > +static int FUNC(quantization_params)(CodedBitstreamContext *ctx, > RWContext *rw, > > + VPXRangeCoder *c, > > +VP8RawFrameHeader *current) { #ifdef READ > > + vp8_rac_unsigned(7, base_qindex); > > + > > + if (cbs_vp8_rac_get(c, 1)) > > + vp8_rac_signed(4, y1dc_delta_q); > > + > > + if (cbs_vp8_rac_get(c, 1)) > > + vp8_rac_signed(4, y2dc_delta_q); > > + > > + if (cbs_vp8_rac_get(c, 1)) > > + vp8_rac_signed(4, y2ac_delta_q); > > + > > + if (cbs_vp8_rac_get(c, 1)) > > + vp8_rac_signed(4, uvdc_delta_q); > > + > > + if (cbs_vp8_rac_get(c, 1)) > > + vp8_rac_signed(4, uvac_delta_q); > > + > > + return 0; > > +#else > > + return AVERROR_PATCHWELCOME; > > +#endif > > +} > > + > > +static int FUNC(update_token_probs)(CodedBitstreamContext *ctx, > RWContext *rw, > > + VPXRangeCoder *c, > > + VP8RawFrameHeader *current) { > > +#ifdef READ > > + int i, j, k, l; > > + > > + for (i = 0; i < 4; ++i) { > > + for (j = 0; j < 8; ++j) { > > + for (k = 0; k < 3; ++k) { > > + for (l = 0; l < 11; ++l) > > + if (vpx_rac_get_prob_branchy( > > + c, vp8_token_update_probs[i][j][k][l])) > > + cbs_vp8_rac_get(c, 8); > > + } > > + } > > + } > > + > > + return 0; > > +#else > > + return AVERROR_PATCHWELCOME; > > +#endif > > +} > > + > > +static int FUNC(update_mv_probs)(CodedBitstreamContext *ctx, RWContext > *rw, > > + VPXRangeCoder *c, VP8RawFrameHeader > > +*current) { #ifdef READ > > + int i, j; > > + > > + for (i = 0; i < 2; ++i) { > > + for (j = 0; j < 19; ++j) > > + if (cbs_vp8_rac_get(c, 1)) > > + cbs_vp8_rac_get(c, 7); > > + } > > + > > + return 0; > > +#else > > + return AVERROR_PATCHWELCOME; > > +#endif > > +} > > + > > +static int FUNC(frame_tag)(CodedBitstreamContext *ctx, RWContext *rw, > > + VP8RawFrameHeader *current) { #ifdef READ > > + int err; > > + > > + vp8_f(1, frame_type); > > + vp8_f(3, profile); > > + vp8_f(1, show_frame); > > + vp8_f(19, first_partition_length_in_bytes); > > + > > + if (current->frame_type == VP8_KEY_FRAME) { > > + fixed(8, start_code_0, VP8_START_CODE_0); > > + fixed(8, start_code_1, VP8_START_CODE_1); > > + fixed(8, start_code_2, VP8_START_CODE_2); > > + > > + vp8_f(14, width); > > + vp8_f(2, horizontal_scale); > > + vp8_f(14, height); > > + vp8_f(2, vertical_scale); > > + } > > + > > + return 0; > > +#else > > + return AVERROR_PATCHWELCOME; > > +#endif > > +} > > + > > +static int FUNC(frame_header)(CodedBitstreamContext *ctx, RWContext *rw, > > + VP8RawFrameHeader *current) { #ifdef > > +READ > > + CodedBitstreamVP8Context *vp8 = ctx->priv_data; > > + VPXRangeCoder *c = &vp8->c; > > + int header_size; > > + int pos; > > + int err; > > + > > + pos = get_bits_count(rw); > > + av_assert0(pos % 8 == 0); > > + pos /= 8; > > + > > + header_size = current->first_partition_length_in_bytes; > > + > > + err = ff_vpx_init_range_decoder(c, rw->buffer + pos, header_size); > > + if (err < 0) > > + return err; > > + > > + if (current->frame_type == VP8_KEY_FRAME) { > > + vp8_rac_unsigned(1, color_space); > > + vp8_rac_unsigned(1, clamping_type); > > + } > > + > > + vp8_rac_unsigned(1, segmentation_enabled); > > + if (current->segmentation_enabled) > > + CHECK(FUNC(update_segmentation)(ctx, rw, c, current)); > > + > > + vp8_rac_unsigned(1, loop_filter_type); > > + vp8_rac_unsigned(6, loop_filter_level); > > + vp8_rac_unsigned(3, loop_filter_sharpness); > > + > > + CHECK(FUNC(mode_ref_lf_deltas)(ctx, rw, c, current)); > > + > > + vp8_rac_unsigned(2, log2_token_partitions); > > + > > + CHECK(FUNC(quantization_params)(ctx, rw, c, current)); > > + > > + if (current->frame_type != VP8_KEY_FRAME) { > > + vp8_rac_unsigned(1, refresh_golden_frame); > > + vp8_rac_unsigned(1, refresh_alternate_frame); > > + if (!current->refresh_golden_frame) > > + cbs_vp8_rac_get(c, 2); > > + if (!current->refresh_alternate_frame) > > + cbs_vp8_rac_get(c, 2); > > + vp8_rac_unsigned(1, ref_frame_sign_bias_golden); > > + vp8_rac_unsigned(1, ref_frame_sign_bias_alternate); > > + } > > + vp8_rac_unsigned(1, refresh_entropy_probs); > > + if (current->frame_type != VP8_KEY_FRAME) > > + vp8_rac_unsigned(1, refresh_last_frame); > > + > > + CHECK(FUNC(update_token_probs)(ctx, rw, c, current)); > > + > > + vp8_rac_unsigned(1, mb_no_skip_coeff); > > + if (current->mb_no_skip_coeff) > > + vp8_rac_unsigned(8, prob_skip_false); > > + > > + if (current->frame_type != VP8_KEY_FRAME) { > > + int i; > > + > > + vp8_rac_unsigned(8, prob_intra); > > + vp8_rac_unsigned(8, prob_last); > > + vp8_rac_unsigned(8, prob_golden); > > + > > + // intra_16x16_prob > > + if (cbs_vp8_rac_get(c, 1)) > > + for (i = 0; i < 4; i++) > > + cbs_vp8_rac_get(c, 8); > > + > > + // intra_chroma_prob > > + if (cbs_vp8_rac_get(c, 1)) > > + for (i = 0; i < 4; i++) > > + cbs_vp8_rac_get(c, 8); > > + > > + CHECK(FUNC(update_mv_probs)(ctx, rw, c, current)); > > + } > > + > > + return 0; > > +#else > > + return AVERROR_PATCHWELCOME; > > +#endif > > +} > > + > > +static int FUNC(trailing_bits)(CodedBitstreamContext *ctx, RWContext > > +*rw) { #ifdef READ > > + int err; > > + > > + while (byte_alignment(rw) != 0) > > + fixed(1, zero_bit, 0); > > + > > + return 0; > > +#else > > + return AVERROR_PATCHWELCOME; > > +#endif > > +} > > + > > +static int FUNC(frame)(CodedBitstreamContext *ctx, RWContext *rw, > > + VP8RawFrame *current) { #ifdef READ > > + int err; > > + > > + HEADER("Frame"); > > + > > + CHECK(FUNC(frame_tag)(ctx, rw, ¤t->header)); > > + CHECK(FUNC(frame_header)(ctx, rw, ¤t->header)); > > + CHECK(FUNC(trailing_bits)(ctx, rw)); > > + > > + return 0; > > +#else > > + return AVERROR_PATCHWELCOME; > > +#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". _______________________________________________ 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".