* [FFmpeg-devel] [PATCH] avcodec/lagarith: use VLC for probe code length
@ 2023-09-13 21:19 Paul B Mahol
2023-09-14 1:28 ` Andreas Rheinhardt
0 siblings, 1 reply; 6+ messages in thread
From: Paul B Mahol @ 2023-09-13 21:19 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1: Type: text/plain, Size: 16 bytes --]
Patch Attached.
[-- Attachment #2: 0002-avcodec-lagarith-use-VLC-for-prob-code-length.patch --]
[-- Type: text/x-patch, Size: 3505 bytes --]
From cd774229dcf32cb44455031663b4357f0597b4c0 Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Tue, 12 Sep 2023 01:29:55 +0200
Subject: [PATCH] avcodec/lagarith: use VLC for prob code length
Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
libavcodec/lagarith.c | 57 ++++++++++++++++++++++++++++++-------------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/libavcodec/lagarith.c b/libavcodec/lagarith.c
index ebc1f7613a..31cad907c0 100644
--- a/libavcodec/lagarith.c
+++ b/libavcodec/lagarith.c
@@ -27,6 +27,9 @@
#include <inttypes.h>
+#include "libavutil/mem_internal.h"
+#include "libavutil/thread.h"
+
#include "avcodec.h"
#include "codec_internal.h"
#include "get_bits.h"
@@ -35,6 +38,8 @@
#include "lossless_videodsp.h"
#include "thread.h"
+#define VLC_BITS 11
+
enum LagarithFrameType {
FRAME_RAW = 1, /**< uncompressed */
FRAME_U_RGB24 = 2, /**< unaligned RGB24 */
@@ -56,6 +61,35 @@ typedef struct LagarithContext {
int zeros_rem; /**< number of zero bytes remaining to output */
} LagarithContext;
+static VLC lag_tab;
+
+static const uint8_t lag_bits[] = {
+ 7, 7, 7, 2, 7, 3, 4, 5, 6, 7, 7, 7, 7, 7, 6, 7, 4, 5, 7, 7, 7, 7,
+ 5, 6, 7, 7, 7, 7, 7, 7, 6, 7, 7, 7, 7, 7, 6, 7, 7, 7, 7, 7, 7, 7,
+ 7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
+};
+
+static const uint8_t lag_codes[] = {
+ 0x00, 0x01, 0x02, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x04, 0x05,
+ 0x08, 0x09, 0x0A, 0x0B, 0x0B, 0x0B, 0x0B, 0x10, 0x11, 0x12, 0x13,
+ 0x13, 0x13, 0x14, 0x15, 0x20, 0x21, 0x22, 0x23, 0x23, 0x24, 0x25,
+ 0x28, 0x29, 0x2A, 0x2B, 0x2B, 0x40, 0x41, 0x42, 0x43, 0x44, 0x45,
+ 0x48, 0x49, 0x4A, 0x4B, 0x50, 0x51, 0x52, 0x53, 0x54, 0x55,
+};
+
+static const uint8_t lag_symbols[] = {
+ -1, 20, 12, 0, 12, 1, 2, 4, 7, 7, 28, 4, 25, 17,
+ 10, 17, 3, 6, 2, 23, 15, 15, 5, 9, 10, 31, 1, 22,
+ 14, 14, 8, 9, 30, 6, 27, 19, 11, 19, 0, 21, 13, 13,
+ 8, 29, 5, 26, 18, 18, 3, 24, 16, 16, 11, 32,
+};
+
+static av_cold void lag_init_static_data(void)
+{
+ VLC_INIT_SPARSE_STATIC(&lag_tab, VLC_BITS, FF_ARRAY_ELEMS(lag_bits),
+ lag_bits, 1, 1, lag_codes, 1, 1, lag_symbols, 1, 1, 2048);
+}
+
/**
* Compute the 52-bit mantissa of 1/(double)denom.
* This crazy format uses floats in an entropy coder and we have to match x86
@@ -101,23 +135,10 @@ static uint8_t lag_calc_zero_run(int8_t x)
static int lag_decode_prob(GetBitContext *gb, uint32_t *value)
{
- static const uint8_t series[] = { 1, 2, 3, 5, 8, 13, 21 };
- int i;
- int bit = 0;
- int bits = 0;
- int prevbit = 0;
- unsigned val;
-
- for (i = 0; i < 7; i++) {
- if (prevbit && bit)
- break;
- prevbit = bit;
- bit = get_bits1(gb);
- if (bit && !prevbit)
- bits += series[i];
- }
- bits--;
- if (bits < 0 || bits > 31) {
+ unsigned val, bits;
+
+ bits = get_vlc2(gb, lag_tab.table, VLC_BITS, 3);
+ if (bits > 31) {
*value = 0;
return AVERROR_INVALIDDATA;
} else if (bits == 0) {
@@ -720,10 +741,12 @@ static int lag_decode_frame(AVCodecContext *avctx, AVFrame *p,
static av_cold int lag_decode_init(AVCodecContext *avctx)
{
+ static AVOnce init_static_once = AV_ONCE_INIT;
LagarithContext *l = avctx->priv_data;
l->avctx = avctx;
ff_llviddsp_init(&l->llviddsp);
+ ff_thread_once(&init_static_once, lag_init_static_data);
return 0;
}
--
2.39.1
[-- Attachment #3: 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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/lagarith: use VLC for probe code length
2023-09-13 21:19 [FFmpeg-devel] [PATCH] avcodec/lagarith: use VLC for probe code length Paul B Mahol
@ 2023-09-14 1:28 ` Andreas Rheinhardt
2023-09-14 6:57 ` Paul B Mahol
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Rheinhardt @ 2023-09-14 1:28 UTC (permalink / raw)
To: ffmpeg-devel
Paul B Mahol:
>
> +#include "libavutil/mem_internal.h"
I don't get what this header is needed for. You are not adding anything
ALIGNED and this file does not require it.
> +#define VLC_BITS 11
> +
> enum LagarithFrameType {
> FRAME_RAW = 1, /**< uncompressed */
> FRAME_U_RGB24 = 2, /**< unaligned RGB24 */
> @@ -56,6 +61,35 @@ typedef struct LagarithContext {
> int zeros_rem; /**< number of zero bytes remaining to output */
> } LagarithContext;
>
> +static VLC lag_tab;
> +
> +static const uint8_t lag_bits[] = {
> + 7, 7, 7, 2, 7, 3, 4, 5, 6, 7, 7, 7, 7, 7, 6, 7, 4, 5, 7, 7, 7, 7,
> + 5, 6, 7, 7, 7, 7, 7, 7, 6, 7, 7, 7, 7, 7, 6, 7, 7, 7, 7, 7, 7, 7,
> + 7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
> +};
> +
> +static const uint8_t lag_codes[] = {
> + 0x00, 0x01, 0x02, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x04, 0x05,
> + 0x08, 0x09, 0x0A, 0x0B, 0x0B, 0x0B, 0x0B, 0x10, 0x11, 0x12, 0x13,
> + 0x13, 0x13, 0x14, 0x15, 0x20, 0x21, 0x22, 0x23, 0x23, 0x24, 0x25,
> + 0x28, 0x29, 0x2A, 0x2B, 0x2B, 0x40, 0x41, 0x42, 0x43, 0x44, 0x45,
> + 0x48, 0x49, 0x4A, 0x4B, 0x50, 0x51, 0x52, 0x53, 0x54, 0x55,
> +};
> +
> +static const uint8_t lag_symbols[] = {
> + -1, 20, 12, 0, 12, 1, 2, 4, 7, 7, 28, 4, 25, 17,
> + 10, 17, 3, 6, 2, 23, 15, 15, 5, 9, 10, 31, 1, 22,
> + 14, 14, 8, 9, 30, 6, 27, 19, 11, 19, 0, 21, 13, 13,
> + 8, 29, 5, 26, 18, 18, 3, 24, 16, 16, 11, 32,
> +};
> +
> +static av_cold void lag_init_static_data(void)
> +{
> + VLC_INIT_SPARSE_STATIC(&lag_tab, VLC_BITS, FF_ARRAY_ELEMS(lag_bits),
> + lag_bits, 1, 1, lag_codes, 1, 1, lag_symbols, 1, 1, 2048);
> +}
> +
If the longest code has seven bits, why are you using 11 bits for the
VLC? This just wastes cache/memory.
Apart from that:
Your first entry will be converted to an uint8_t of 255 (and give a
-Woverflow warning when said warning is enabled) and this is what
get_vlc2() will return for it, i.e. it will trigger the bits > 31 check
and error out, which is probably what you intend it to do given that
this behaviour coincides with the current behaviour.
But the more natural way for VLCs to achieve this is to actually not add
invalid codes. get_vlc2() will then return -1 for them; this means that
the check for invalid values becomes "bits < 0" (in which case the flags
from this comparison can be reused for the "bits == 0" check).
In contrast to the current code and your proposed patch no bits would be
consumed upon encountering such an invalid code though. But it seems to
me that the we error out anyway and the state of the GetBitContext
afterwards doesn't matter.
If you were to use the init_from_lengths variants, you would have to use
negative bitlengths for the invalid values.
- Andreas
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/lagarith: use VLC for probe code length
2023-09-14 1:28 ` Andreas Rheinhardt
@ 2023-09-14 6:57 ` Paul B Mahol
2023-09-14 13:35 ` Andreas Rheinhardt
2023-09-16 12:21 ` Paul B Mahol
0 siblings, 2 replies; 6+ messages in thread
From: Paul B Mahol @ 2023-09-14 6:57 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1: Type: text/plain, Size: 20 bytes --]
New patch attached.
[-- Attachment #2: 0001-avcodec-lagarith-use-VLC-for-prob-code-length.patch --]
[-- Type: text/x-patch, Size: 3453 bytes --]
From e7d44f39eba93d567bc8e70826793c3e032311b1 Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Tue, 12 Sep 2023 01:29:55 +0200
Subject: [PATCH] avcodec/lagarith: use VLC for prob code length
Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
libavcodec/lagarith.c | 56 ++++++++++++++++++++++++++++++-------------
1 file changed, 39 insertions(+), 17 deletions(-)
diff --git a/libavcodec/lagarith.c b/libavcodec/lagarith.c
index ebc1f7613a..4de5e96084 100644
--- a/libavcodec/lagarith.c
+++ b/libavcodec/lagarith.c
@@ -27,6 +27,8 @@
#include <inttypes.h>
+#include "libavutil/thread.h"
+
#include "avcodec.h"
#include "codec_internal.h"
#include "get_bits.h"
@@ -35,6 +37,8 @@
#include "lossless_videodsp.h"
#include "thread.h"
+#define VLC_BITS 7
+
enum LagarithFrameType {
FRAME_RAW = 1, /**< uncompressed */
FRAME_U_RGB24 = 2, /**< unaligned RGB24 */
@@ -56,6 +60,35 @@ typedef struct LagarithContext {
int zeros_rem; /**< number of zero bytes remaining to output */
} LagarithContext;
+static VLC lag_tab;
+
+static const uint8_t lag_bits[] = {
+ 7, 7, 2, 7, 3, 4, 5, 6, 7, 7, 7, 7, 7, 6, 7, 4, 5, 7, 7, 7, 7,
+ 5, 6, 7, 7, 7, 7, 7, 7, 6, 7, 7, 7, 7, 7, 6, 7, 7, 7, 7, 7, 7, 7,
+ 7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
+};
+
+static const uint8_t lag_codes[] = {
+ 0x01, 0x02, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x04, 0x05,
+ 0x08, 0x09, 0x0A, 0x0B, 0x0B, 0x0B, 0x0B, 0x10, 0x11, 0x12, 0x13,
+ 0x13, 0x13, 0x14, 0x15, 0x20, 0x21, 0x22, 0x23, 0x23, 0x24, 0x25,
+ 0x28, 0x29, 0x2A, 0x2B, 0x2B, 0x40, 0x41, 0x42, 0x43, 0x44, 0x45,
+ 0x48, 0x49, 0x4A, 0x4B, 0x50, 0x51, 0x52, 0x53, 0x54, 0x55,
+};
+
+static const uint8_t lag_symbols[] = {
+ 20, 12, 0, 12, 1, 2, 4, 7, 7, 28, 4, 25, 17,
+ 10, 17, 3, 6, 2, 23, 15, 15, 5, 9, 10, 31, 1, 22,
+ 14, 14, 8, 9, 30, 6, 27, 19, 11, 19, 0, 21, 13, 13,
+ 8, 29, 5, 26, 18, 18, 3, 24, 16, 16, 11, 32,
+};
+
+static av_cold void lag_init_static_data(void)
+{
+ VLC_INIT_SPARSE_STATIC(&lag_tab, VLC_BITS, FF_ARRAY_ELEMS(lag_bits),
+ lag_bits, 1, 1, lag_codes, 1, 1, lag_symbols, 1, 1, 128);
+}
+
/**
* Compute the 52-bit mantissa of 1/(double)denom.
* This crazy format uses floats in an entropy coder and we have to match x86
@@ -101,23 +134,10 @@ static uint8_t lag_calc_zero_run(int8_t x)
static int lag_decode_prob(GetBitContext *gb, uint32_t *value)
{
- static const uint8_t series[] = { 1, 2, 3, 5, 8, 13, 21 };
- int i;
- int bit = 0;
- int bits = 0;
- int prevbit = 0;
- unsigned val;
-
- for (i = 0; i < 7; i++) {
- if (prevbit && bit)
- break;
- prevbit = bit;
- bit = get_bits1(gb);
- if (bit && !prevbit)
- bits += series[i];
- }
- bits--;
- if (bits < 0 || bits > 31) {
+ unsigned val, bits;
+
+ bits = get_vlc2(gb, lag_tab.table, VLC_BITS, 3);
+ if (bits > 31) {
*value = 0;
return AVERROR_INVALIDDATA;
} else if (bits == 0) {
@@ -720,10 +740,12 @@ static int lag_decode_frame(AVCodecContext *avctx, AVFrame *p,
static av_cold int lag_decode_init(AVCodecContext *avctx)
{
+ static AVOnce init_static_once = AV_ONCE_INIT;
LagarithContext *l = avctx->priv_data;
l->avctx = avctx;
ff_llviddsp_init(&l->llviddsp);
+ ff_thread_once(&init_static_once, lag_init_static_data);
return 0;
}
--
2.39.1
[-- Attachment #3: 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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/lagarith: use VLC for probe code length
2023-09-14 6:57 ` Paul B Mahol
@ 2023-09-14 13:35 ` Andreas Rheinhardt
2023-09-14 13:35 ` Paul B Mahol
2023-09-16 12:21 ` Paul B Mahol
1 sibling, 1 reply; 6+ messages in thread
From: Andreas Rheinhardt @ 2023-09-14 13:35 UTC (permalink / raw)
To: ffmpeg-devel
Paul B Mahol:
> + unsigned val, bits;
> +
> + bits = get_vlc2(gb, lag_tab.table, VLC_BITS, 3);
> + if (bits > 31) {
> *value = 0;
> return AVERROR_INVALIDDATA;
Using a signed int and checking for zero is simpler.
And the depth of this VLC is 1, not 3.
- Andreas
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/lagarith: use VLC for probe code length
2023-09-14 13:35 ` Andreas Rheinhardt
@ 2023-09-14 13:35 ` Paul B Mahol
0 siblings, 0 replies; 6+ messages in thread
From: Paul B Mahol @ 2023-09-14 13:35 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Thu, Sep 14, 2023 at 3:34 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:
> Paul B Mahol:
> > + unsigned val, bits;
> > +
> > + bits = get_vlc2(gb, lag_tab.table, VLC_BITS, 3);
> > + if (bits > 31) {
> > *value = 0;
> > return AVERROR_INVALIDDATA;
>
> Using a signed int and checking for zero is simpler.
> And the depth of this VLC is 1, not 3.
>
There is 32 symbol value bit length, that current code ignores too.
>
> - Andreas
>
> _______________________________________________
> 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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/lagarith: use VLC for probe code length
2023-09-14 6:57 ` Paul B Mahol
2023-09-14 13:35 ` Andreas Rheinhardt
@ 2023-09-16 12:21 ` Paul B Mahol
1 sibling, 0 replies; 6+ messages in thread
From: Paul B Mahol @ 2023-09-16 12:21 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Will apply soon.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-16 12:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-13 21:19 [FFmpeg-devel] [PATCH] avcodec/lagarith: use VLC for probe code length Paul B Mahol
2023-09-14 1:28 ` Andreas Rheinhardt
2023-09-14 6:57 ` Paul B Mahol
2023-09-14 13:35 ` Andreas Rheinhardt
2023-09-14 13:35 ` Paul B Mahol
2023-09-16 12:21 ` Paul B Mahol
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git