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 D0BD946428 for ; Mon, 16 Oct 2023 21:13:58 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C381E68C9D3; Tue, 17 Oct 2023 00:13:55 +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 9D91D68C82C for ; Tue, 17 Oct 2023 00:13:48 +0300 (EEST) Received: by mail-wm1-f49.google.com with SMTP id 5b1f17b1804b1-406650da82bso45992915e9.3 for ; Mon, 16 Oct 2023 14:13:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jkqxz-net.20230601.gappssmtp.com; s=20230601; t=1697490827; x=1698095627; 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=vYrTDEIGdMUpd/Y6+2ToLyCEFDCyDwN4XBnhQ434aZY=; b=DO/DYQFE5Mnrhn5Yo0OwZxM6L4GIrfK/vxEFA+O25yqTqxltKoVrVFVF7Wk7j9NHf8 Bijbyc1YU/QX/tp/3fmXTMM+cTGBc2WwheIONO9aKePhOPvhLe0aVkzRGK94BwAOlpTn oef5qlii0tUA7dQX434QnVW9pcw6yh+WfEuMKtw0z9wMbDor34ySAsSMJUa13OOKRfGp 5DOOOMaWB4qAQ5pgkm3ln2N/ocIZLKDIzZ64OPRcTmTM8Rml7A3p/f025xKE3gtjxTP6 lljNNOKMR0h+q+nrq7ynmz9TUy/FuwlfN8oC1yFbe89+WHhkVMju4eTNuuwfqT9n2kyg aXjg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697490827; x=1698095627; 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=vYrTDEIGdMUpd/Y6+2ToLyCEFDCyDwN4XBnhQ434aZY=; b=wX6eCX/IuRRT9wjgQO1zkz/Tmy6iRzgZW0SnXeKzaZ349rbRN3tQsYQhsxn9ESqfvx Ji11vPKEBlcRBbLrHiM0XHp9vNkA480r093BYNKdGRnYDO2marwWFeuELinxY530TjhL iP+EhUlqAtuSyimRjE9zDjp3bYFeSTR3KMRJgyzgeXqI3wE5AdIrGpSY867K8oafqVIH RW5DRIOmqObZ+FZ+sY4FH88GFomIkWnQFxlw3fdyAbUvUKVbQCPhjp2ykmC2VeQXwpq1 KJl/vZESigPRXVLcCgCs5SFZAeuo9B8dvyvEf07jiEUgDjgdcO7BEXMQfGgJiZ8vPEHy NAxA== X-Gm-Message-State: AOJu0Yz/RbN82g9Hz0n135Eq+//ogH/79wrEnApuGAJgj1zwdPH/P2uA ZtJY8lF58sQlEOBT/3h7HusAR3zVfiwX6/YiFgw= X-Google-Smtp-Source: AGHT+IGrJJht87531UmJih5M1KQlFqRnp+Hk+JIkqSDMSaoiM42LKKMW9qhuX6Be0CLF6oSkKFQxYA== X-Received: by 2002:a05:600c:1910:b0:406:5396:9f9e with SMTP id j16-20020a05600c191000b0040653969f9emr233200wmq.32.1697490826674; Mon, 16 Oct 2023 14:13:46 -0700 (PDT) Received: from [192.168.0.15] (cpc92320-cmbg19-2-0-cust383.5-4.cable.virginm.net. [82.13.65.128]) by smtp.gmail.com with ESMTPSA id bi13-20020a05600c3d8d00b003fee567235bsm8085479wmb.1.2023.10.16.14.13.45 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 16 Oct 2023 14:13:46 -0700 (PDT) Message-ID: Date: Mon, 16 Oct 2023 22:13:58 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: From: Mark Thompson In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH v1] avcodec/cbs: Keep ff_cbs_trace_syntax_element 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 10/10/2023 04:30, Dai, Jianhui J wrote: >> -----Original Message----- >> From: Dai, Jianhui J >> Sent: Tuesday, October 10, 2023 10:57 AM >> To: ffmpeg-devel@ffmpeg.org >> Subject: [PATCH v1] avcodec/cbs: Keep ff_cbs_trace_syntax_element >> >> Split ff_cbs_trace_syntax_element from ff_cbs_trace_read_log to decouple the >> tracing from GetBitContext. This allows CBS implementations that do not have a >> GetBitContext to directly use ff_cbs_trace_syntax_element to trace syntax >> elements. >> >> Signed-off-by: Jianhui Dai >> --- >> libavcodec/cbs.c | 41 +++++++++++++++++++++++++-------------- >> libavcodec/cbs_internal.h | 4 ++++ >> 2 files changed, 30 insertions(+), 15 deletions(-) >> >> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c index cdd7adebeb..2f2cfcfb31 >> 100644 >> --- a/libavcodec/cbs.c >> +++ b/libavcodec/cbs.c >> @@ -498,26 +498,18 @@ void ff_cbs_trace_header(CodedBitstreamContext >> *ctx, >> av_log(ctx->log_ctx, ctx->trace_level, "%s\n", name); } >> >> -void ff_cbs_trace_read_log(void *trace_context, >> - GetBitContext *gbc, int length, >> - const char *str, const int *subscripts, >> - int64_t value) >> +void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx, int position, >> + const char *str, const int *subscripts, >> + const char *bits, int64_t value) >> { >> - CodedBitstreamContext *ctx = trace_context; >> char name[256]; >> - char bits[256]; >> size_t name_len, bits_len; >> int pad, subs, i, j, k, n; >> - int position; >> - >> - av_assert0(value >= INT_MIN && value <= UINT32_MAX); >> >> - position = get_bits_count(gbc); >> + if (!ctx->trace_enable) >> + return; >> >> - av_assert0(length < 256); >> - for (i = 0; i < length; i++) >> - bits[i] = get_bits1(gbc) ? '1' : '0'; >> - bits[length] = 0; >> + av_assert0(value >= INT_MIN && value <= UINT32_MAX); >> >> subs = subscripts ? subscripts[0] : 0; >> n = 0; >> @@ -545,7 +537,7 @@ void ff_cbs_trace_read_log(void *trace_context, >> av_assert0(n == subs); >> >> name_len = strlen(name); >> - bits_len = length; >> + bits_len = strlen(bits); >> >> if (name_len + bits_len > 60) >> pad = bits_len + 2; >> @@ -556,6 +548,25 @@ void ff_cbs_trace_read_log(void *trace_context, >> position, name, pad, bits, value); } >> >> +void ff_cbs_trace_read_log(void *trace_context, >> + GetBitContext *gbc, int length, >> + const char *str, const int *subscripts, >> + int64_t value) { >> + CodedBitstreamContext *ctx = trace_context; >> + char bits[256]; >> + int position; >> + >> + position = get_bits_count(gbc); >> + >> + av_assert0(length < 256); >> + for (int i = 0; i < length; i++) >> + bits[i] = get_bits1(gbc) ? '1' : '0'; >> + bits[length] = 0; >> + >> + ff_cbs_trace_syntax_element(ctx, position, str, subscripts, bits, >> +value); } >> + >> void ff_cbs_trace_write_log(void *trace_context, >> PutBitContext *pbc, int length, >> const char *str, const int *subscripts, diff --git >> a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h index >> 07220f1f3e..ff90ce467d 100644 >> --- a/libavcodec/cbs_internal.h >> +++ b/libavcodec/cbs_internal.h >> @@ -158,6 +158,10 @@ typedef struct CodedBitstreamType { void >> ff_cbs_trace_header(CodedBitstreamContext *ctx, >> const char *name); >> >> +void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx, int position, >> + const char *name, const int *subscripts, >> + const char *bitstring, int64_t value); >> + >> >> // Helper functions for read/write of common bitstream elements, including // >> generation of trace output. The simple functions are equivalent to > > @Mark Thompson > Could you please take a look if it's a valid change based on your last refactor? > It's intended to use for the reviewing cbs_vp8 patch. The simple answer here is to forge a GetBitContext pointing at the right place, like the write tracing does. However: for your VP8 case, I'm a bit unclear why it isn't using get_bits already? The setup seems to be to stop normal parsing at the end of the uncompressed header and then read bytes through a pointer for the range coder rather than continuing with the bit reader. If the range decoder used the GetBitContext to read the input instead of reading bytes from the array then your problem would be solved. Doing that would also allow it to renormalise directly after each read rather than reading by bytes, so the actual bits consumed for each symbol would be visible in tracing. (I admit I'm not very clear what your motivation for making a read-only CBS implementation for VP8 is, and that might guide the best answer. If it's tracing then showing the consumed bits precisely seems useful, but if it's something else then that's less relevant.) 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".