Re: [FFmpeg-devel] [PATCH 5/9] avcodec/vlc: Pass VLC_MULTI_ELEM directly not by pointer
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 > > > > --- > > > >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 signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 5/9] avcodec/vlc: Pass VLC_MULTI_ELEM directly not by pointer
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 --- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 5/9] avcodec/vlc: Pass VLC_MULTI_ELEM directly not by pointer
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 > > --- > > 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. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 5/9] avcodec/vlc: Pass VLC_MULTI_ELEM directly not by pointer
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 --- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 5/9] avcodec/vlc: Pass VLC_MULTI_ELEM directly not by pointer
This makes the code more testable as uninitialized fields are 0
and not random values from the last call
Signed-off-by: Michael Niedermayer
---
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, );
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, );
+ 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-requ...@ffmpeg.org with subject "unsubscribe".
