[FFmpeg-devel] [PATCH 1/9] avcodec/vlc: merge lost 16bit end of array check

[FFmpeg-devel] [PATCH 1/9] avcodec/vlc: merge lost 16bit end of array check

[Michael Niedermayer]

Also cleanup related code

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/vlc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
index e4bbf2945e1..4f62eddf0fb 100644
--- a/libavcodec/vlc.c
+++ b/libavcodec/vlc.c
@@ -358,8 +358,8 @@ static void add_level(VLC_MULTI_ELEM *table, const int nb_elems,
                       const int minlen, const int max,
                       unsigned* levelcnt, VLC_MULTI_ELEM *info)
 {
-    if (nb_elems > 256 && curlevel > 2)
-        return; // No room
+    int is16bit = nb_elems>256;
+    int max_symbols = VLC_MULTI_MAX_SYMBOLS >> is16bit;
     for (int i = num-1; i > max; i--) {
         for (int j = 0; j < 2; j++) {
             int newlimit, sym;
@@ -373,7 +373,7 @@ static void add_level(VLC_MULTI_ELEM *table, const int nb_elems,
             code = curcode + (buf[t].code >> curlen);
             newlimit = curlimit - l;
             l  += curlen;
-            if (nb_elems>256) AV_WN16(info->val+2*curlevel, sym);
+            if (is16bit) AV_WN16(info->val+2*curlevel, sym);
             else info->val[curlevel] = sym&0xFF;
 
             if (curlevel) { // let's not add single entries
@@ -386,7 +386,7 @@ static void add_level(VLC_MULTI_ELEM *table, const int nb_elems,
                 levelcnt[curlevel-1]++;
             }
 
-            if (curlevel+1 < VLC_MULTI_MAX_SYMBOLS && newlimit >= minlen) {
+            if (curlevel+1 < max_symbols && newlimit >= minlen) {
                 add_level(table, nb_elems, num, numbits, buf,
                           code, l, newlimit, curlevel+1,
                           minlen, max, levelcnt, info);
-- 
2.17.1

_______________________________________________
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] [PATCH 2/9] avcodec/vlc: dont pass nb_elems into multi vlc code

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/vlc.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
index 4f62eddf0fb..77860430861 100644
--- a/libavcodec/vlc.c
+++ b/libavcodec/vlc.c
@@ -350,7 +350,7 @@ fail:
     return AVERROR_INVALIDDATA;
 }
 
-static void add_level(VLC_MULTI_ELEM *table, const int nb_elems,
+static void add_level(VLC_MULTI_ELEM *table, const int is16bit,
                       const int num, const int numbits,
                       const VLCcode *buf,
                       uint32_t curcode, int curlen,
@@ -358,7 +358,6 @@ static void add_level(VLC_MULTI_ELEM *table, const int nb_elems,
                       const int minlen, const int max,
                       unsigned* levelcnt, VLC_MULTI_ELEM *info)
 {
-    int is16bit = nb_elems>256;
     int max_symbols = VLC_MULTI_MAX_SYMBOLS >> is16bit;
     for (int i = num-1; i > max; i--) {
         for (int j = 0; j < 2; j++) {
@@ -387,7 +386,7 @@ static void add_level(VLC_MULTI_ELEM *table, const int nb_elems,
             }
 
             if (curlevel+1 < max_symbols && newlimit >= minlen) {
-                add_level(table, nb_elems, num, numbits, buf,
+                add_level(table, is16bit, num, numbits, buf,
                           code, l, newlimit, curlevel+1,
                           minlen, max, levelcnt, info);
             }
@@ -396,7 +395,7 @@ static void add_level(VLC_MULTI_ELEM *table, const int nb_elems,
 }
 
 static int vlc_multi_gen(VLC_MULTI_ELEM *table, const VLC *single,
-                         const int nb_elems, const int nb_codes, const int numbits,
+                         const int is16bit, const int nb_codes, const int numbits,
                          VLCcode *buf, void *logctx)
 {
     int minbits, maxbits, max = nb_codes-1;
@@ -424,7 +423,7 @@ static int vlc_multi_gen(VLC_MULTI_ELEM *table, const VLC *single,
         AV_WN16(table[j].val, single->table[j].sym);
     }
 
-    add_level(table, nb_elems, nb_codes, numbits, buf,
+    add_level(table, is16bit, nb_codes, numbits, buf,
               0, 0, FFMIN(maxbits, numbits), 0, minbits, max, count, &info);
 
     av_log(logctx, AV_LOG_DEBUG, "Joint: %d/%d/%d/%d/%d codes min=%ubits max=%u\n",
@@ -480,7 +479,7 @@ int ff_vlc_init_multi_from_lengths(VLC *vlc, VLC_MULTI *multi, int nb_bits, int
     ret = vlc_common_end(vlc, nb_bits, j, buf, flags, buf);
     if (ret < 0)
         goto fail;
-    ret = vlc_multi_gen(multi->table, vlc, nb_elems, j, nb_bits, buf, logctx);
+    ret = vlc_multi_gen(multi->table, vlc, nb_elems > 256, j, nb_bits, buf, logctx);
     if (buf != localbuf)
         av_free(buf);
     return ret;
-- 
2.17.1

_______________________________________________
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] [PATCH 3/9] avcodec/vlc: Skip subtable entries in multi VLC

[Michael Niedermayer]

These entries do not correspond to VLC symbols that can be used
they do corrupt various variables like min/max bits

This also no longer assumes that there is a single non subtable
entry
Probably fixes some infinite loops too

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/vlc.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
index 77860430861..65883a506ff 100644
--- a/libavcodec/vlc.c
+++ b/libavcodec/vlc.c
@@ -401,15 +401,23 @@ static int vlc_multi_gen(VLC_MULTI_ELEM *table, const VLC *single,
     int minbits, maxbits, max = nb_codes-1;
     unsigned count[VLC_MULTI_MAX_SYMBOLS-1] = { 0, };
     VLC_MULTI_ELEM info = { { 0, }, 0, 0, };
+    int count0 = 0;
 
-    minbits = buf[0].bits;
-    maxbits = buf[0].bits;
+    for (int j = 0; j < 1<<numbits; j++) {
+        if (single->table[j].len > 0) {
+            count0 ++;
+            j += (1 << (numbits - single->table[j].len)) - 1;
+        }
+    }
+
+    minbits = 32;
+    maxbits = 0;
 
-    for (int n = 1; n < nb_codes; n++) {
+    for (int n = nb_codes - count0; n < nb_codes; n++) {
         minbits = FFMIN(minbits, buf[n].bits);
         maxbits = FFMAX(maxbits, buf[n].bits);
     }
-    maxbits = FFMIN(maxbits, numbits);
+    av_assert0(maxbits <= numbits);
 
     while (max >= nb_codes/2) {
         if (buf[max].bits+minbits > maxbits)
-- 
2.17.1

_______________________________________________
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] [PATCH 4/9] avcodec/vlc: Replace mysterious max computation code in multi vlc

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/vlc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
index 65883a506ff..9b7a42f79a3 100644
--- a/libavcodec/vlc.c
+++ b/libavcodec/vlc.c
@@ -359,7 +359,7 @@ static void add_level(VLC_MULTI_ELEM *table, const int is16bit,
                       unsigned* levelcnt, VLC_MULTI_ELEM *info)
 {
     int max_symbols = VLC_MULTI_MAX_SYMBOLS >> is16bit;
-    for (int i = num-1; i > max; i--) {
+    for (int i = num-1; i >= max; i--) {
         for (int j = 0; j < 2; j++) {
             int newlimit, sym;
             int t = j ? i-1 : i;
@@ -398,7 +398,7 @@ static int vlc_multi_gen(VLC_MULTI_ELEM *table, const VLC *single,
                          const int is16bit, const int nb_codes, const int numbits,
                          VLCcode *buf, void *logctx)
 {
-    int minbits, maxbits, max = nb_codes-1;
+    int minbits, maxbits, max;
     unsigned count[VLC_MULTI_MAX_SYMBOLS-1] = { 0, };
     VLC_MULTI_ELEM info = { { 0, }, 0, 0, };
     int count0 = 0;
@@ -419,10 +419,13 @@ static int vlc_multi_gen(VLC_MULTI_ELEM *table, const VLC *single,
     }
     av_assert0(maxbits <= numbits);
 
-    while (max >= nb_codes/2) {
-        if (buf[max].bits+minbits > maxbits)
+    for (max = nb_codes; max > nb_codes - count0; max--) {
+        // We can only add a code that fits with the shortest other code into the table
+        // We assume the table is sorted by bits and we skip subtables which from our
+        // point of view are basically random corrupted entries
+        // If we have not a single useable vlc we end with max = nb_codes
+        if (buf[max - 1].bits+minbits > numbits)
             break;
-        max--;
     }
 
     for (int j = 0; j < 1<<numbits; j++) {
-- 
2.17.1

_______________________________________________
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] [PATCH 5/9] avcodec/vlc: Pass VLC_MULTI_ELEM directly not by pointer

[Michael Niedermayer]

This makes the code more testable as uninitialized fields are 0
and not random values from the last call

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/vlc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
index 9b7a42f79a3..4adec2da705 100644
--- a/libavcodec/vlc.c
+++ b/libavcodec/vlc.c
@@ -356,7 +356,7 @@ static void add_level(VLC_MULTI_ELEM *table, const int is16bit,
                       uint32_t curcode, int curlen,
                       int curlimit, int curlevel,
                       const int minlen, const int max,
-                      unsigned* levelcnt, VLC_MULTI_ELEM *info)
+                      unsigned* levelcnt, VLC_MULTI_ELEM info)
 {
     int max_symbols = VLC_MULTI_MAX_SYMBOLS >> is16bit;
     for (int i = num-1; i >= max; i--) {
@@ -372,16 +372,16 @@ static void add_level(VLC_MULTI_ELEM *table, const int is16bit,
             code = curcode + (buf[t].code >> curlen);
             newlimit = curlimit - l;
             l  += curlen;
-            if (is16bit) AV_WN16(info->val+2*curlevel, sym);
-            else info->val[curlevel] = sym&0xFF;
+            if (is16bit) AV_WN16(info.val+2*curlevel, sym);
+            else info.val[curlevel] = sym&0xFF;
 
             if (curlevel) { // let's not add single entries
                 uint32_t val = code >> (32 - numbits);
                 uint32_t  nb = val + (1U << (numbits - l));
-                info->len = l;
-                info->num = curlevel+1;
+                info.len = l;
+                info.num = curlevel+1;
                 for (; val < nb; val++)
-                    AV_COPY64(table+val, info);
+                    AV_COPY64(table+val, &info);
                 levelcnt[curlevel-1]++;
             }
 
@@ -435,7 +435,7 @@ static int vlc_multi_gen(VLC_MULTI_ELEM *table, const VLC *single,
     }
 
     add_level(table, is16bit, nb_codes, numbits, buf,
-              0, 0, FFMIN(maxbits, numbits), 0, minbits, max, count, &info);
+              0, 0, FFMIN(maxbits, numbits), 0, minbits, max, count, info);
 
     av_log(logctx, AV_LOG_DEBUG, "Joint: %d/%d/%d/%d/%d codes min=%ubits max=%u\n",
            count[0], count[1], count[2], count[3], count[4], minbits, max);
-- 
2.17.1

_______________________________________________
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] [PATCH 6/9] avcodec/vlc: Remove mysterious jitter loop in multi VLC

[Michael Niedermayer]

No difference in my testcase in the tables content

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/vlc.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
index 4adec2da705..4c96fcddc9b 100644
--- a/libavcodec/vlc.c
+++ b/libavcodec/vlc.c
@@ -360,16 +360,14 @@ static void add_level(VLC_MULTI_ELEM *table, const int is16bit,
 {
     int max_symbols = VLC_MULTI_MAX_SYMBOLS >> is16bit;
     for (int i = num-1; i >= max; i--) {
-        for (int j = 0; j < 2; j++) {
             int newlimit, sym;
-            int t = j ? i-1 : i;
-            int l = buf[t].bits;
+            int l = buf[i].bits;
             uint32_t code;
 
-            sym = buf[t].symbol;
+            sym = buf[i].symbol;
             if (l >= curlimit)
                 return;
-            code = curcode + (buf[t].code >> curlen);
+            code = curcode + (buf[i].code >> curlen);
             newlimit = curlimit - l;
             l  += curlen;
             if (is16bit) AV_WN16(info.val+2*curlevel, sym);
@@ -390,7 +388,6 @@ static void add_level(VLC_MULTI_ELEM *table, const int is16bit,
                           code, l, newlimit, curlevel+1,
                           minlen, max, levelcnt, info);
             }
-        }
     }
 }
 
-- 
2.17.1

_______________________________________________
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] [PATCH 7/9] Revert "avcodec/vlc: add correct upper limit for recursive function"

[Michael Niedermayer]

This reverts commit fa20f5cd9e131f22da06ef57bf5aedd87ff51a90.
---
 libavcodec/vlc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
index 4c96fcddc9b..79544006677 100644
--- a/libavcodec/vlc.c
+++ b/libavcodec/vlc.c
@@ -432,7 +432,7 @@ static int vlc_multi_gen(VLC_MULTI_ELEM *table, const VLC *single,
     }
 
     add_level(table, is16bit, nb_codes, numbits, buf,
-              0, 0, FFMIN(maxbits, numbits), 0, minbits, max, count, info);
+              0, 0, numbits, 0, minbits, max, count, info);
 
     av_log(logctx, AV_LOG_DEBUG, "Joint: %d/%d/%d/%d/%d codes min=%ubits max=%u\n",
            count[0], count[1], count[2], count[3], count[4], minbits, max);
-- 
2.17.1

_______________________________________________
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] [PATCH 8/9] Revert "avcodec/vlc: fix off by one in limit check for multi"

[Michael Niedermayer]

This with the last revert results in a table with additional entries

This reverts commit b23eaf968e375f2d38865d90a788221caead3324.
---
 libavcodec/vlc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
index 79544006677..ceabeba5408 100644
--- a/libavcodec/vlc.c
+++ b/libavcodec/vlc.c
@@ -365,7 +365,7 @@ static void add_level(VLC_MULTI_ELEM *table, const int is16bit,
             uint32_t code;
 
             sym = buf[i].symbol;
-            if (l >= curlimit)
+            if (l > curlimit)
                 return;
             code = curcode + (buf[i].code >> curlen);
             newlimit = curlimit - l;
-- 
2.17.1

_______________________________________________
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] [PATCH 9/9] avcodec/vlc: simplify min/maxbits in multi VLC

[Michael Niedermayer]

nothing uses maxbits, so its removed
minbits is just the last entry (verified with assert on fate)

This basically reverts 58d9b5caf3d332c6495f9af437158bf45531a05e for the minbits computation

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/vlc.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
index ceabeba5408..de16424c93d 100644
--- a/libavcodec/vlc.c
+++ b/libavcodec/vlc.c
@@ -395,7 +395,7 @@ static int vlc_multi_gen(VLC_MULTI_ELEM *table, const VLC *single,
                          const int is16bit, const int nb_codes, const int numbits,
                          VLCcode *buf, void *logctx)
 {
-    int minbits, maxbits, max;
+    int minbits, max;
     unsigned count[VLC_MULTI_MAX_SYMBOLS-1] = { 0, };
     VLC_MULTI_ELEM info = { { 0, }, 0, 0, };
     int count0 = 0;
@@ -407,14 +407,9 @@ static int vlc_multi_gen(VLC_MULTI_ELEM *table, const VLC *single,
         }
     }
 
-    minbits = 32;
-    maxbits = 0;
-
-    for (int n = nb_codes - count0; n < nb_codes; n++) {
-        minbits = FFMIN(minbits, buf[n].bits);
-        maxbits = FFMAX(maxbits, buf[n].bits);
-    }
-    av_assert0(maxbits <= numbits);
+    //This is only correct if count0 > 0 and the table is sorted
+    //minbits is not used if count0 == 0 and other parts assume the table is sorted too
+    minbits = buf[nb_codes - 1].bits;
 
     for (max = nb_codes; max > nb_codes - count0; max--) {
         // We can only add a code that fits with the shortest other code into the table
-- 
2.17.1

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

Re: [FFmpeg-devel] [PATCH 1/9] avcodec/vlc: merge lost 16bit end of array check

[Paul B Mahol]

I have no time to review these hacks.

Make sure you do not break >8 bit support in utvideo and magicyuv decoders.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 5/9] avcodec/vlc: Pass VLC_MULTI_ELEM directly not by pointer

[Leo Izen]

On 10/22/23 17:51, Michael Niedermayer wrote:
> This makes the code more testable as uninitialized fields are 0
> and not random values from the last call
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavcodec/vlc.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
> index 9b7a42f79a3..4adec2da705 100644
> --- a/libavcodec/vlc.c
> +++ b/libavcodec/vlc.c
> @@ -356,7 +356,7 @@ static void add_level(VLC_MULTI_ELEM *table, const int is16bit,
>                         uint32_t curcode, int curlen,
>                         int curlimit, int curlevel,
>                         const int minlen, const int max,
> -                      unsigned* levelcnt, VLC_MULTI_ELEM *info)
> +                      unsigned* levelcnt, VLC_MULTI_ELEM info)


Is passing a struct by value advisable? Did you benchmark this? How does 
it compare to memset(info, 0, sizeof(*info))?

- Leo Izen (Traneptora)

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

Re: [FFmpeg-devel] [PATCH 5/9] avcodec/vlc: Pass VLC_MULTI_ELEM directly not by pointer

[Michael Niedermayer]

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

On Mon, Oct 23, 2023 at 02:10:35AM -0400, Leo Izen wrote:
> On 10/22/23 17:51, Michael Niedermayer wrote:
> > This makes the code more testable as uninitialized fields are 0
> > and not random values from the last call
> > 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/vlc.c | 14 +++++++-------
> >   1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
> > index 9b7a42f79a3..4adec2da705 100644
> > --- a/libavcodec/vlc.c
> > +++ b/libavcodec/vlc.c
> > @@ -356,7 +356,7 @@ static void add_level(VLC_MULTI_ELEM *table, const int is16bit,
> >                         uint32_t curcode, int curlen,
> >                         int curlimit, int curlevel,
> >                         const int minlen, const int max,
> > -                      unsigned* levelcnt, VLC_MULTI_ELEM *info)
> > +                      unsigned* levelcnt, VLC_MULTI_ELEM info)
> 
> 
> Is passing a struct by value advisable? Did you benchmark this? How does it
> compare to memset(info, 0, sizeof(*info))?

The struct is 8 bytes, a pointer on 64bit arch is also 8byte

I did not benchmark, I think this code doesnt run that many iterations
(when its not buggy), I mean each iteration adds a entry to the table
and the table will normally be designed to fit in cache and its only
for initializing.

do you still want me to bechmark this ?

thx

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

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.

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

Re: [FFmpeg-devel] [PATCH 1/9] avcodec/vlc: merge lost 16bit end of array check

[Michael Niedermayer]

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

On Mon, Oct 23, 2023 at 01:02:16AM +0200, Paul B Mahol wrote:
[...]

> Make sure you do not break >8 bit support in utvideo and magicyuv decoders.

Make sure the fate tests cover these cases

thx

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

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides

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

Re: [FFmpeg-devel] [PATCH 5/9] avcodec/vlc: Pass VLC_MULTI_ELEM directly not by pointer

[Leo Izen]

On 10/23/23 12:04, Michael Niedermayer wrote:
> On Mon, Oct 23, 2023 at 02:10:35AM -0400, Leo Izen wrote:
>> On 10/22/23 17:51, Michael Niedermayer wrote:
>>> This makes the code more testable as uninitialized fields are 0
>>> and not random values from the last call
>>>
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>    libavcodec/vlc.c | 14 +++++++-------
>>>    1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
>>> index 9b7a42f79a3..4adec2da705 100644
>>> --- a/libavcodec/vlc.c
>>> +++ b/libavcodec/vlc.c
>>> @@ -356,7 +356,7 @@ static void add_level(VLC_MULTI_ELEM *table, const int is16bit,
>>>                          uint32_t curcode, int curlen,
>>>                          int curlimit, int curlevel,
>>>                          const int minlen, const int max,
>>> -                      unsigned* levelcnt, VLC_MULTI_ELEM *info)
>>> +                      unsigned* levelcnt, VLC_MULTI_ELEM info)
>>
>>
>> Is passing a struct by value advisable? Did you benchmark this? How does it
>> compare to memset(info, 0, sizeof(*info))?
> 
> The struct is 8 bytes, a pointer on 64bit arch is also 8byte
> 
> I did not benchmark, I think this code doesnt run that many iterations
> (when its not buggy), I mean each iteration adds a entry to the table
> and the table will normally be designed to fit in cache and its only
> for initializing.
> 
> do you still want me to bechmark this ?
> 
> thx
> 

If the struct is only 8 bytes it's probably not necessary.

- Leo Izen (Traneptora)

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

Re: [FFmpeg-devel] [PATCH 1/9] avcodec/vlc: merge lost 16bit end of array check

[Michael Niedermayer]

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

On Sun, Oct 22, 2023 at 11:51:05PM +0200, Michael Niedermayer wrote:
> Also cleanup related code
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/vlc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

will apply patches 1 and 2

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

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus

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

Re: [FFmpeg-devel] [PATCH 5/9] avcodec/vlc: Pass VLC_MULTI_ELEM directly not by pointer

[Michael Niedermayer]

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

On Tue, Oct 24, 2023 at 05:54:37AM -0400, Leo Izen wrote:
> On 10/23/23 12:04, Michael Niedermayer wrote:
> > On Mon, Oct 23, 2023 at 02:10:35AM -0400, Leo Izen wrote:
> > > On 10/22/23 17:51, Michael Niedermayer wrote:
> > > > This makes the code more testable as uninitialized fields are 0
> > > > and not random values from the last call
> > > > 
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >    libavcodec/vlc.c | 14 +++++++-------
> > > >    1 file changed, 7 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/libavcodec/vlc.c b/libavcodec/vlc.c
> > > > index 9b7a42f79a3..4adec2da705 100644
> > > > --- a/libavcodec/vlc.c
> > > > +++ b/libavcodec/vlc.c
> > > > @@ -356,7 +356,7 @@ static void add_level(VLC_MULTI_ELEM *table, const int is16bit,
> > > >                          uint32_t curcode, int curlen,
> > > >                          int curlimit, int curlevel,
> > > >                          const int minlen, const int max,
> > > > -                      unsigned* levelcnt, VLC_MULTI_ELEM *info)
> > > > +                      unsigned* levelcnt, VLC_MULTI_ELEM info)
> > > 
> > > 
> > > Is passing a struct by value advisable? Did you benchmark this? How does it
> > > compare to memset(info, 0, sizeof(*info))?
> > 
> > The struct is 8 bytes, a pointer on 64bit arch is also 8byte
> > 
> > I did not benchmark, I think this code doesnt run that many iterations
> > (when its not buggy), I mean each iteration adds a entry to the table
> > and the table will normally be designed to fit in cache and its only
> > for initializing.
> > 
> > do you still want me to bechmark this ?
> > 
> > thx
> > 
> 
> If the struct is only 8 bytes it's probably not necessary.

will apply patches 3-5

thx

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

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras

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