| Message ID | 20201110104851.321029-5-andreas.rheinhardt@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | VLC, esp. init_vlc patches | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
I do not think this is a good direction. On Tue, Nov 10, 2020 at 11:50 AM Andreas Rheinhardt < andreas.rheinhardt@gmail.com> wrote: > Right now the allocated size of the VLC table of a static VLC has to > exactly match the size actually used for the VLC: If it is not enough, > abort is called; if it is more than enough, an error message is > emitted. This is no problem when one wants to initialize an individual > VLC via one of the INIT_VLC macros as one just hardcodes the needed > size. Yet it is an obstacle when one wants to initialize several VLCs > in a loop as one then needs to add an array for the sizes/offsets of > the VLC tables (unless max_depth of all arrays is one in which case > the sizes are derivable from the number of bits used). > > Yet said size array is not necessary if one removes the warning for too > big buffers. The reason is that the amount of entries needed for the > table is of course generated as a byproduct of initializing the VLC. > So one can proceed as follows: > > static VLC vlcs[NUM]; > static VLC_TYPE vlc_table[BUF_SIZE][2]; > > for (int i = 0, offset = 0; i < NUM; i++) { > vlcs[i].table = &vlc_table[offset]; > vlcs[i].table_allocated = BUF_SIZE - offset; > init_vlc(); > offset += vlcs[i].table_size; > } > > Of course, BUF_SIZE should be equal to the number of entries actually > needed. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavcodec/bitstream.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c > index 03d39ad88c..4ffec7e17a 100644 > --- a/libavcodec/bitstream.c > +++ b/libavcodec/bitstream.c > @@ -281,9 +281,6 @@ static int vlc_common_end(VLC *vlc, int nb_bits, int > nb_codes, VLCcode *codes, > int ret = build_table(vlc, nb_bits, nb_codes, codes, flags); > > if (flags & INIT_VLC_USE_NEW_STATIC) { > - if(vlc->table_size != vlc->table_allocated) > - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", > vlc->table_size, vlc->table_allocated); > - > av_assert0(ret >= 0); > *vlc_arg = *vlc; > } else { > -- > 2.25.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".
Paul B Mahol: > I do not think this is a good direction. > Why? > On Tue, Nov 10, 2020 at 11:50 AM Andreas Rheinhardt < > andreas.rheinhardt@gmail.com> wrote: > >> Right now the allocated size of the VLC table of a static VLC has to >> exactly match the size actually used for the VLC: If it is not enough, >> abort is called; if it is more than enough, an error message is >> emitted. This is no problem when one wants to initialize an individual >> VLC via one of the INIT_VLC macros as one just hardcodes the needed >> size. Yet it is an obstacle when one wants to initialize several VLCs >> in a loop as one then needs to add an array for the sizes/offsets of >> the VLC tables (unless max_depth of all arrays is one in which case >> the sizes are derivable from the number of bits used). >> >> Yet said size array is not necessary if one removes the warning for too >> big buffers. The reason is that the amount of entries needed for the >> table is of course generated as a byproduct of initializing the VLC. >> So one can proceed as follows: >> >> static VLC vlcs[NUM]; >> static VLC_TYPE vlc_table[BUF_SIZE][2]; >> >> for (int i = 0, offset = 0; i < NUM; i++) { >> vlcs[i].table = &vlc_table[offset]; >> vlcs[i].table_allocated = BUF_SIZE - offset; >> init_vlc(); >> offset += vlcs[i].table_size; >> } >> >> Of course, BUF_SIZE should be equal to the number of entries actually >> needed. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >> --- >> libavcodec/bitstream.c | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c >> index 03d39ad88c..4ffec7e17a 100644 >> --- a/libavcodec/bitstream.c >> +++ b/libavcodec/bitstream.c >> @@ -281,9 +281,6 @@ static int vlc_common_end(VLC *vlc, int nb_bits, int >> nb_codes, VLCcode *codes, >> int ret = build_table(vlc, nb_bits, nb_codes, codes, flags); >> >> if (flags & INIT_VLC_USE_NEW_STATIC) { >> - if(vlc->table_size != vlc->table_allocated) >> - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", >> vlc->table_size, vlc->table_allocated); >> - >> av_assert0(ret >= 0); >> *vlc_arg = *vlc; >> } else { >> -- >> 2.25.1 >>
Because it is a bad idea. No need to change code that worked for ages. On Tue, Nov 10, 2020 at 12:26 PM Andreas Rheinhardt < andreas.rheinhardt@gmail.com> wrote: > Paul B Mahol: > > I do not think this is a good direction. > > > > Why? > > > On Tue, Nov 10, 2020 at 11:50 AM Andreas Rheinhardt < > > andreas.rheinhardt@gmail.com> wrote: > > > >> Right now the allocated size of the VLC table of a static VLC has to > >> exactly match the size actually used for the VLC: If it is not enough, > >> abort is called; if it is more than enough, an error message is > >> emitted. This is no problem when one wants to initialize an individual > >> VLC via one of the INIT_VLC macros as one just hardcodes the needed > >> size. Yet it is an obstacle when one wants to initialize several VLCs > >> in a loop as one then needs to add an array for the sizes/offsets of > >> the VLC tables (unless max_depth of all arrays is one in which case > >> the sizes are derivable from the number of bits used). > >> > >> Yet said size array is not necessary if one removes the warning for too > >> big buffers. The reason is that the amount of entries needed for the > >> table is of course generated as a byproduct of initializing the VLC. > >> So one can proceed as follows: > >> > >> static VLC vlcs[NUM]; > >> static VLC_TYPE vlc_table[BUF_SIZE][2]; > >> > >> for (int i = 0, offset = 0; i < NUM; i++) { > >> vlcs[i].table = &vlc_table[offset]; > >> vlcs[i].table_allocated = BUF_SIZE - offset; > >> init_vlc(); > >> offset += vlcs[i].table_size; > >> } > >> > >> Of course, BUF_SIZE should be equal to the number of entries actually > >> needed. > >> > >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > >> --- > >> libavcodec/bitstream.c | 3 --- > >> 1 file changed, 3 deletions(-) > >> > >> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c > >> index 03d39ad88c..4ffec7e17a 100644 > >> --- a/libavcodec/bitstream.c > >> +++ b/libavcodec/bitstream.c > >> @@ -281,9 +281,6 @@ static int vlc_common_end(VLC *vlc, int nb_bits, int > >> nb_codes, VLCcode *codes, > >> int ret = build_table(vlc, nb_bits, nb_codes, codes, flags); > >> > >> if (flags & INIT_VLC_USE_NEW_STATIC) { > >> - if(vlc->table_size != vlc->table_allocated) > >> - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", > >> vlc->table_size, vlc->table_allocated); > >> - > >> av_assert0(ret >= 0); > >> *vlc_arg = *vlc; > >> } else { > >> -- > >> 2.25.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".
Paul B Mahol: > Because it is a bad idea. And I still like to hear a reason for this. > No need to change code that worked for ages. > It allows to remove useless tables and it simplifies making more VLCs that should be static static. > On Tue, Nov 10, 2020 at 12:26 PM Andreas Rheinhardt < > andreas.rheinhardt@gmail.com> wrote: > >> Paul B Mahol: >>> I do not think this is a good direction. >>> >> >> Why? >> >>> On Tue, Nov 10, 2020 at 11:50 AM Andreas Rheinhardt < >>> andreas.rheinhardt@gmail.com> wrote: >>> >>>> Right now the allocated size of the VLC table of a static VLC has to >>>> exactly match the size actually used for the VLC: If it is not enough, >>>> abort is called; if it is more than enough, an error message is >>>> emitted. This is no problem when one wants to initialize an individual >>>> VLC via one of the INIT_VLC macros as one just hardcodes the needed >>>> size. Yet it is an obstacle when one wants to initialize several VLCs >>>> in a loop as one then needs to add an array for the sizes/offsets of >>>> the VLC tables (unless max_depth of all arrays is one in which case >>>> the sizes are derivable from the number of bits used). >>>> >>>> Yet said size array is not necessary if one removes the warning for too >>>> big buffers. The reason is that the amount of entries needed for the >>>> table is of course generated as a byproduct of initializing the VLC. >>>> So one can proceed as follows: >>>> >>>> static VLC vlcs[NUM]; >>>> static VLC_TYPE vlc_table[BUF_SIZE][2]; >>>> >>>> for (int i = 0, offset = 0; i < NUM; i++) { >>>> vlcs[i].table = &vlc_table[offset]; >>>> vlcs[i].table_allocated = BUF_SIZE - offset; >>>> init_vlc(); >>>> offset += vlcs[i].table_size; >>>> } >>>> >>>> Of course, BUF_SIZE should be equal to the number of entries actually >>>> needed. >>>> >>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >>>> --- >>>> libavcodec/bitstream.c | 3 --- >>>> 1 file changed, 3 deletions(-) >>>> >>>> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c >>>> index 03d39ad88c..4ffec7e17a 100644 >>>> --- a/libavcodec/bitstream.c >>>> +++ b/libavcodec/bitstream.c >>>> @@ -281,9 +281,6 @@ static int vlc_common_end(VLC *vlc, int nb_bits, int >>>> nb_codes, VLCcode *codes, >>>> int ret = build_table(vlc, nb_bits, nb_codes, codes, flags); >>>> >>>> if (flags & INIT_VLC_USE_NEW_STATIC) { >>>> - if(vlc->table_size != vlc->table_allocated) >>>> - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", >>>> vlc->table_size, vlc->table_allocated); >>>> - >>>> av_assert0(ret >= 0); >>>> *vlc_arg = *vlc; >>>> } else { >>>> -- >>>> 2.25.1 >>>> >>
On Tue, Nov 10, 2020 at 12:31 PM Andreas Rheinhardt < andreas.rheinhardt@gmail.com> wrote: > Paul B Mahol: > > Because it is a bad idea. > > And I still like to hear a reason for this. > > Code is there so correct intended size is always allocated. > > No need to change code that worked for ages. > > > > It allows to remove useless tables and it simplifies making more VLCs > that should be static static. > What is static static? How does it allows to remove useless tables? > > > On Tue, Nov 10, 2020 at 12:26 PM Andreas Rheinhardt < > > andreas.rheinhardt@gmail.com> wrote: > > > >> Paul B Mahol: > >>> I do not think this is a good direction. > >>> > >> > >> Why? > >> > >>> On Tue, Nov 10, 2020 at 11:50 AM Andreas Rheinhardt < > >>> andreas.rheinhardt@gmail.com> wrote: > >>> > >>>> Right now the allocated size of the VLC table of a static VLC has to > >>>> exactly match the size actually used for the VLC: If it is not enough, > >>>> abort is called; if it is more than enough, an error message is > >>>> emitted. This is no problem when one wants to initialize an individual > >>>> VLC via one of the INIT_VLC macros as one just hardcodes the needed > >>>> size. Yet it is an obstacle when one wants to initialize several VLCs > >>>> in a loop as one then needs to add an array for the sizes/offsets of > >>>> the VLC tables (unless max_depth of all arrays is one in which case > >>>> the sizes are derivable from the number of bits used). > >>>> > >>>> Yet said size array is not necessary if one removes the warning for > too > >>>> big buffers. The reason is that the amount of entries needed for the > >>>> table is of course generated as a byproduct of initializing the VLC. > >>>> So one can proceed as follows: > >>>> > >>>> static VLC vlcs[NUM]; > >>>> static VLC_TYPE vlc_table[BUF_SIZE][2]; > >>>> > >>>> for (int i = 0, offset = 0; i < NUM; i++) { > >>>> vlcs[i].table = &vlc_table[offset]; > >>>> vlcs[i].table_allocated = BUF_SIZE - offset; > >>>> init_vlc(); > >>>> offset += vlcs[i].table_size; > >>>> } > >>>> > >>>> Of course, BUF_SIZE should be equal to the number of entries actually > >>>> needed. > >>>> > >>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > >>>> --- > >>>> libavcodec/bitstream.c | 3 --- > >>>> 1 file changed, 3 deletions(-) > >>>> > >>>> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c > >>>> index 03d39ad88c..4ffec7e17a 100644 > >>>> --- a/libavcodec/bitstream.c > >>>> +++ b/libavcodec/bitstream.c > >>>> @@ -281,9 +281,6 @@ static int vlc_common_end(VLC *vlc, int nb_bits, > int > >>>> nb_codes, VLCcode *codes, > >>>> int ret = build_table(vlc, nb_bits, nb_codes, codes, flags); > >>>> > >>>> if (flags & INIT_VLC_USE_NEW_STATIC) { > >>>> - if(vlc->table_size != vlc->table_allocated) > >>>> - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", > >>>> vlc->table_size, vlc->table_allocated); > >>>> - > >>>> av_assert0(ret >= 0); > >>>> *vlc_arg = *vlc; > >>>> } else { > >>>> -- > >>>> 2.25.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".
Paul B Mahol: > On Tue, Nov 10, 2020 at 12:31 PM Andreas Rheinhardt < > andreas.rheinhardt@gmail.com> wrote: > >> Paul B Mahol: >>> Because it is a bad idea. >> >> And I still like to hear a reason for this. >> >> > Code is there so correct intended size is always allocated. > No. There are two ways to create a VLC: With the INIT_VLC_USE_NEW_STATIC flag set and without it. If it is unset, ff_init_vlc_sparse() (and ff_init_vlc_from_lengths(), too) allocates the needed space for the VLC table (actually, it overallocates) and the VLC must in turn be freed via ff_free_vlc(). In this case the whole given VLC is treated as uninitialized, i.e. vlc->table_allocated is ignored. If it is set, then vlc->table must point to an array VLC_TYPE[][2] with at least vlc->table_allocated elements and vlc->table_allocated must be sufficient for the whole VLC: This buffer will never be (re)allocated; it actually points to static storage and not to anything that is heap-allocated. Given the error message that this commit intends to remove there is currently a requirement for table_allocated to be exactly equal to the actually required size. And this requires tables of the sizes if VLCs are initialized in a loop (alternatively one can of course unroll the loop and hardcode the values via one of the INIT_VLC macros). > >>> No need to change code that worked for ages. >>> >> >> It allows to remove useless tables and it simplifies making more VLCs >> that should be static static. >> > > What is static static? How does it allows to remove useless tables? > And this requirement is just a pointless straitjacket. If one drops it, one can remove the useless tables by using the pattern mentioned in the commit message. Several commits of this series provide examples for this; e.g. #12 (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272139.html), #40 (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272167.html), #53 (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272179.html), #57 (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272183.html) or #88 (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272215.html). These are only examples where existing offsets tables were removed; making VLCs static like in #68 (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272195.html; making these VLCs static is especially beneficial for a multithreaded decoder like this one so that the VLCs can be shared between all threads) would be more complicated if the requirement of always using exactly the required size were not dropped. > >> >>> On Tue, Nov 10, 2020 at 12:26 PM Andreas Rheinhardt < >>> andreas.rheinhardt@gmail.com> wrote: >>> >>>> Paul B Mahol: >>>>> I do not think this is a good direction. >>>>> >>>> >>>> Why? >>>> >>>>> On Tue, Nov 10, 2020 at 11:50 AM Andreas Rheinhardt < >>>>> andreas.rheinhardt@gmail.com> wrote: >>>>> >>>>>> Right now the allocated size of the VLC table of a static VLC has to >>>>>> exactly match the size actually used for the VLC: If it is not enough, >>>>>> abort is called; if it is more than enough, an error message is >>>>>> emitted. This is no problem when one wants to initialize an individual >>>>>> VLC via one of the INIT_VLC macros as one just hardcodes the needed >>>>>> size. Yet it is an obstacle when one wants to initialize several VLCs >>>>>> in a loop as one then needs to add an array for the sizes/offsets of >>>>>> the VLC tables (unless max_depth of all arrays is one in which case >>>>>> the sizes are derivable from the number of bits used). >>>>>> >>>>>> Yet said size array is not necessary if one removes the warning for >> too >>>>>> big buffers. The reason is that the amount of entries needed for the >>>>>> table is of course generated as a byproduct of initializing the VLC. >>>>>> So one can proceed as follows: >>>>>> >>>>>> static VLC vlcs[NUM]; >>>>>> static VLC_TYPE vlc_table[BUF_SIZE][2]; >>>>>> >>>>>> for (int i = 0, offset = 0; i < NUM; i++) { >>>>>> vlcs[i].table = &vlc_table[offset]; >>>>>> vlcs[i].table_allocated = BUF_SIZE - offset; >>>>>> init_vlc(); >>>>>> offset += vlcs[i].table_size; >>>>>> } >>>>>> >>>>>> Of course, BUF_SIZE should be equal to the number of entries actually >>>>>> needed. >>>>>> >>>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >>>>>> --- >>>>>> libavcodec/bitstream.c | 3 --- >>>>>> 1 file changed, 3 deletions(-) >>>>>> >>>>>> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c >>>>>> index 03d39ad88c..4ffec7e17a 100644 >>>>>> --- a/libavcodec/bitstream.c >>>>>> +++ b/libavcodec/bitstream.c >>>>>> @@ -281,9 +281,6 @@ static int vlc_common_end(VLC *vlc, int nb_bits, >> int >>>>>> nb_codes, VLCcode *codes, >>>>>> int ret = build_table(vlc, nb_bits, nb_codes, codes, flags); >>>>>> >>>>>> if (flags & INIT_VLC_USE_NEW_STATIC) { >>>>>> - if(vlc->table_size != vlc->table_allocated) >>>>>> - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", >>>>>> vlc->table_size, vlc->table_allocated); >>>>>> - >>>>>> av_assert0(ret >= 0); >>>>>> *vlc_arg = *vlc; >>>>>> } else { >>>>>> -- >>>>>> 2.25.1 >>>>>> >>>> >>
Than how one is supposed to guess correct size to put when this log is gone? On Tue, Nov 10, 2020 at 1:15 PM Andreas Rheinhardt < andreas.rheinhardt@gmail.com> wrote: > Paul B Mahol: > > On Tue, Nov 10, 2020 at 12:31 PM Andreas Rheinhardt < > > andreas.rheinhardt@gmail.com> wrote: > > > >> Paul B Mahol: > >>> Because it is a bad idea. > >> > >> And I still like to hear a reason for this. > >> > >> > > Code is there so correct intended size is always allocated. > > > > No. There are two ways to create a VLC: With the INIT_VLC_USE_NEW_STATIC > flag set and without it. > If it is unset, ff_init_vlc_sparse() (and ff_init_vlc_from_lengths(), > too) allocates the needed space for the VLC table (actually, it > overallocates) and the VLC must in turn be freed via ff_free_vlc(). In > this case the whole given VLC is treated as uninitialized, i.e. > vlc->table_allocated is ignored. > > If it is set, then vlc->table must point to an array VLC_TYPE[][2] with > at least vlc->table_allocated elements and vlc->table_allocated must be > sufficient for the whole VLC: This buffer will never be (re)allocated; > it actually points to static storage and not to anything that is > heap-allocated. Given the error message that this commit intends to > remove there is currently a requirement for table_allocated to be > exactly equal to the actually required size. And this requires tables of > the sizes if VLCs are initialized in a loop (alternatively one can of > course unroll the loop and hardcode the values via one of the INIT_VLC > macros). > > > > >>> No need to change code that worked for ages. > >>> > >> > >> It allows to remove useless tables and it simplifies making more VLCs > >> that should be static static. > >> > > > > What is static static? How does it allows to remove useless tables? > > > > And this requirement is just a pointless straitjacket. If one drops it, > one can remove the useless tables by using the pattern mentioned in the > commit message. Several commits of this series provide examples for > this; e.g. #12 > (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272139.html), > #40 > (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272167.html), > #53 > (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272179.html), > #57 > (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272183.html) or > #88 (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272215.html). > > These are only examples where existing offsets tables were removed; > making VLCs static like in #68 > (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272195.html; > making these VLCs static is especially beneficial for a multithreaded > decoder like this one so that the VLCs can be shared between all > threads) would be more complicated if the requirement of always using > exactly the required size were not dropped. > > > > >> > >>> On Tue, Nov 10, 2020 at 12:26 PM Andreas Rheinhardt < > >>> andreas.rheinhardt@gmail.com> wrote: > >>> > >>>> Paul B Mahol: > >>>>> I do not think this is a good direction. > >>>>> > >>>> > >>>> Why? > >>>> > >>>>> On Tue, Nov 10, 2020 at 11:50 AM Andreas Rheinhardt < > >>>>> andreas.rheinhardt@gmail.com> wrote: > >>>>> > >>>>>> Right now the allocated size of the VLC table of a static VLC has to > >>>>>> exactly match the size actually used for the VLC: If it is not > enough, > >>>>>> abort is called; if it is more than enough, an error message is > >>>>>> emitted. This is no problem when one wants to initialize an > individual > >>>>>> VLC via one of the INIT_VLC macros as one just hardcodes the needed > >>>>>> size. Yet it is an obstacle when one wants to initialize several > VLCs > >>>>>> in a loop as one then needs to add an array for the sizes/offsets of > >>>>>> the VLC tables (unless max_depth of all arrays is one in which case > >>>>>> the sizes are derivable from the number of bits used). > >>>>>> > >>>>>> Yet said size array is not necessary if one removes the warning for > >> too > >>>>>> big buffers. The reason is that the amount of entries needed for the > >>>>>> table is of course generated as a byproduct of initializing the VLC. > >>>>>> So one can proceed as follows: > >>>>>> > >>>>>> static VLC vlcs[NUM]; > >>>>>> static VLC_TYPE vlc_table[BUF_SIZE][2]; > >>>>>> > >>>>>> for (int i = 0, offset = 0; i < NUM; i++) { > >>>>>> vlcs[i].table = &vlc_table[offset]; > >>>>>> vlcs[i].table_allocated = BUF_SIZE - offset; > >>>>>> init_vlc(); > >>>>>> offset += vlcs[i].table_size; > >>>>>> } > >>>>>> > >>>>>> Of course, BUF_SIZE should be equal to the number of entries > actually > >>>>>> needed. > >>>>>> > >>>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > >>>>>> --- > >>>>>> libavcodec/bitstream.c | 3 --- > >>>>>> 1 file changed, 3 deletions(-) > >>>>>> > >>>>>> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c > >>>>>> index 03d39ad88c..4ffec7e17a 100644 > >>>>>> --- a/libavcodec/bitstream.c > >>>>>> +++ b/libavcodec/bitstream.c > >>>>>> @@ -281,9 +281,6 @@ static int vlc_common_end(VLC *vlc, int nb_bits, > >> int > >>>>>> nb_codes, VLCcode *codes, > >>>>>> int ret = build_table(vlc, nb_bits, nb_codes, codes, flags); > >>>>>> > >>>>>> if (flags & INIT_VLC_USE_NEW_STATIC) { > >>>>>> - if(vlc->table_size != vlc->table_allocated) > >>>>>> - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", > >>>>>> vlc->table_size, vlc->table_allocated); > >>>>>> - > >>>>>> av_assert0(ret >= 0); > >>>>>> *vlc_arg = *vlc; > >>>>>> } else { > >>>>>> -- > >>>>>> 2.25.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".
Paul B Mahol: > Than how one is supposed to guess correct size to put when this log is gone? > The developer who wants to make VLCs static just has to use a huge array (in order not to run into the abort) and get the value of the final offset parameter (either via av_log or via a debugger); or you just sum the VLC.table_size values before you make the VLCs static. It is no big deal and it is not much different from what it is now. > On Tue, Nov 10, 2020 at 1:15 PM Andreas Rheinhardt < > andreas.rheinhardt@gmail.com> wrote: > >> Paul B Mahol: >>> On Tue, Nov 10, 2020 at 12:31 PM Andreas Rheinhardt < >>> andreas.rheinhardt@gmail.com> wrote: >>> >>>> Paul B Mahol: >>>>> Because it is a bad idea. >>>> >>>> And I still like to hear a reason for this. >>>> >>>> >>> Code is there so correct intended size is always allocated. >>> >> >> No. There are two ways to create a VLC: With the INIT_VLC_USE_NEW_STATIC >> flag set and without it. >> If it is unset, ff_init_vlc_sparse() (and ff_init_vlc_from_lengths(), >> too) allocates the needed space for the VLC table (actually, it >> overallocates) and the VLC must in turn be freed via ff_free_vlc(). In >> this case the whole given VLC is treated as uninitialized, i.e. >> vlc->table_allocated is ignored. >> >> If it is set, then vlc->table must point to an array VLC_TYPE[][2] with >> at least vlc->table_allocated elements and vlc->table_allocated must be >> sufficient for the whole VLC: This buffer will never be (re)allocated; >> it actually points to static storage and not to anything that is >> heap-allocated. Given the error message that this commit intends to >> remove there is currently a requirement for table_allocated to be >> exactly equal to the actually required size. And this requires tables of >> the sizes if VLCs are initialized in a loop (alternatively one can of >> course unroll the loop and hardcode the values via one of the INIT_VLC >> macros). >> >>> >>>>> No need to change code that worked for ages. >>>>> >>>> >>>> It allows to remove useless tables and it simplifies making more VLCs >>>> that should be static static. >>>> >>> >>> What is static static? How does it allows to remove useless tables? >>> >> >> And this requirement is just a pointless straitjacket. If one drops it, >> one can remove the useless tables by using the pattern mentioned in the >> commit message. Several commits of this series provide examples for >> this; e.g. #12 >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272139.html), >> #40 >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272167.html), >> #53 >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272179.html), >> #57 >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272183.html) or >> #88 (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272215.html). >> >> These are only examples where existing offsets tables were removed; >> making VLCs static like in #68 >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272195.html; >> making these VLCs static is especially beneficial for a multithreaded >> decoder like this one so that the VLCs can be shared between all >> threads) would be more complicated if the requirement of always using >> exactly the required size were not dropped. >> >>> >>>> >>>>> On Tue, Nov 10, 2020 at 12:26 PM Andreas Rheinhardt < >>>>> andreas.rheinhardt@gmail.com> wrote: >>>>> >>>>>> Paul B Mahol: >>>>>>> I do not think this is a good direction. >>>>>>> >>>>>> >>>>>> Why? >>>>>> >>>>>>> On Tue, Nov 10, 2020 at 11:50 AM Andreas Rheinhardt < >>>>>>> andreas.rheinhardt@gmail.com> wrote: >>>>>>> >>>>>>>> Right now the allocated size of the VLC table of a static VLC has to >>>>>>>> exactly match the size actually used for the VLC: If it is not >> enough, >>>>>>>> abort is called; if it is more than enough, an error message is >>>>>>>> emitted. This is no problem when one wants to initialize an >> individual >>>>>>>> VLC via one of the INIT_VLC macros as one just hardcodes the needed >>>>>>>> size. Yet it is an obstacle when one wants to initialize several >> VLCs >>>>>>>> in a loop as one then needs to add an array for the sizes/offsets of >>>>>>>> the VLC tables (unless max_depth of all arrays is one in which case >>>>>>>> the sizes are derivable from the number of bits used). >>>>>>>> >>>>>>>> Yet said size array is not necessary if one removes the warning for >>>> too >>>>>>>> big buffers. The reason is that the amount of entries needed for the >>>>>>>> table is of course generated as a byproduct of initializing the VLC. >>>>>>>> So one can proceed as follows: >>>>>>>> >>>>>>>> static VLC vlcs[NUM]; >>>>>>>> static VLC_TYPE vlc_table[BUF_SIZE][2]; >>>>>>>> >>>>>>>> for (int i = 0, offset = 0; i < NUM; i++) { >>>>>>>> vlcs[i].table = &vlc_table[offset]; >>>>>>>> vlcs[i].table_allocated = BUF_SIZE - offset; >>>>>>>> init_vlc(); >>>>>>>> offset += vlcs[i].table_size; >>>>>>>> } >>>>>>>> >>>>>>>> Of course, BUF_SIZE should be equal to the number of entries >> actually >>>>>>>> needed. >>>>>>>> >>>>>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >>>>>>>> --- >>>>>>>> libavcodec/bitstream.c | 3 --- >>>>>>>> 1 file changed, 3 deletions(-) >>>>>>>> >>>>>>>> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c >>>>>>>> index 03d39ad88c..4ffec7e17a 100644 >>>>>>>> --- a/libavcodec/bitstream.c >>>>>>>> +++ b/libavcodec/bitstream.c >>>>>>>> @@ -281,9 +281,6 @@ static int vlc_common_end(VLC *vlc, int nb_bits, >>>> int >>>>>>>> nb_codes, VLCcode *codes, >>>>>>>> int ret = build_table(vlc, nb_bits, nb_codes, codes, flags); >>>>>>>> >>>>>>>> if (flags & INIT_VLC_USE_NEW_STATIC) { >>>>>>>> - if(vlc->table_size != vlc->table_allocated) >>>>>>>> - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", >>>>>>>> vlc->table_size, vlc->table_allocated); >>>>>>>> - >>>>>>>> av_assert0(ret >= 0); >>>>>>>> *vlc_arg = *vlc; >>>>>>>> } else { >>>>>>>> -- >>>>>>>> 2.25.1 >>>>>>>> >>>>>> >>>>
Big disagree, please keep this log intact. Thanks. On Tue, Nov 10, 2020 at 1:31 PM Andreas Rheinhardt < andreas.rheinhardt@gmail.com> wrote: > Paul B Mahol: > > Than how one is supposed to guess correct size to put when this log is > gone? > > > The developer who wants to make VLCs static just has to use a huge array > (in order not to run into the abort) and get the value of the final > offset parameter (either via av_log or via a debugger); or you just sum > the VLC.table_size values before you make the VLCs static. It is no big > deal and it is not much different from what it is now. > > > On Tue, Nov 10, 2020 at 1:15 PM Andreas Rheinhardt < > > andreas.rheinhardt@gmail.com> wrote: > > > >> Paul B Mahol: > >>> On Tue, Nov 10, 2020 at 12:31 PM Andreas Rheinhardt < > >>> andreas.rheinhardt@gmail.com> wrote: > >>> > >>>> Paul B Mahol: > >>>>> Because it is a bad idea. > >>>> > >>>> And I still like to hear a reason for this. > >>>> > >>>> > >>> Code is there so correct intended size is always allocated. > >>> > >> > >> No. There are two ways to create a VLC: With the INIT_VLC_USE_NEW_STATIC > >> flag set and without it. > >> If it is unset, ff_init_vlc_sparse() (and ff_init_vlc_from_lengths(), > >> too) allocates the needed space for the VLC table (actually, it > >> overallocates) and the VLC must in turn be freed via ff_free_vlc(). In > >> this case the whole given VLC is treated as uninitialized, i.e. > >> vlc->table_allocated is ignored. > >> > >> If it is set, then vlc->table must point to an array VLC_TYPE[][2] with > >> at least vlc->table_allocated elements and vlc->table_allocated must be > >> sufficient for the whole VLC: This buffer will never be (re)allocated; > >> it actually points to static storage and not to anything that is > >> heap-allocated. Given the error message that this commit intends to > >> remove there is currently a requirement for table_allocated to be > >> exactly equal to the actually required size. And this requires tables of > >> the sizes if VLCs are initialized in a loop (alternatively one can of > >> course unroll the loop and hardcode the values via one of the INIT_VLC > >> macros). > >> > >>> > >>>>> No need to change code that worked for ages. > >>>>> > >>>> > >>>> It allows to remove useless tables and it simplifies making more VLCs > >>>> that should be static static. > >>>> > >>> > >>> What is static static? How does it allows to remove useless tables? > >>> > >> > >> And this requirement is just a pointless straitjacket. If one drops it, > >> one can remove the useless tables by using the pattern mentioned in the > >> commit message. Several commits of this series provide examples for > >> this; e.g. #12 > >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272139.html), > >> #40 > >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272167.html), > >> #53 > >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272179.html), > >> #57 > >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272183.html) > or > >> #88 ( > https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272215.html). > >> > >> These are only examples where existing offsets tables were removed; > >> making VLCs static like in #68 > >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272195.html; > >> making these VLCs static is especially beneficial for a multithreaded > >> decoder like this one so that the VLCs can be shared between all > >> threads) would be more complicated if the requirement of always using > >> exactly the required size were not dropped. > >> > >>> > >>>> > >>>>> On Tue, Nov 10, 2020 at 12:26 PM Andreas Rheinhardt < > >>>>> andreas.rheinhardt@gmail.com> wrote: > >>>>> > >>>>>> Paul B Mahol: > >>>>>>> I do not think this is a good direction. > >>>>>>> > >>>>>> > >>>>>> Why? > >>>>>> > >>>>>>> On Tue, Nov 10, 2020 at 11:50 AM Andreas Rheinhardt < > >>>>>>> andreas.rheinhardt@gmail.com> wrote: > >>>>>>> > >>>>>>>> Right now the allocated size of the VLC table of a static VLC has > to > >>>>>>>> exactly match the size actually used for the VLC: If it is not > >> enough, > >>>>>>>> abort is called; if it is more than enough, an error message is > >>>>>>>> emitted. This is no problem when one wants to initialize an > >> individual > >>>>>>>> VLC via one of the INIT_VLC macros as one just hardcodes the > needed > >>>>>>>> size. Yet it is an obstacle when one wants to initialize several > >> VLCs > >>>>>>>> in a loop as one then needs to add an array for the sizes/offsets > of > >>>>>>>> the VLC tables (unless max_depth of all arrays is one in which > case > >>>>>>>> the sizes are derivable from the number of bits used). > >>>>>>>> > >>>>>>>> Yet said size array is not necessary if one removes the warning > for > >>>> too > >>>>>>>> big buffers. The reason is that the amount of entries needed for > the > >>>>>>>> table is of course generated as a byproduct of initializing the > VLC. > >>>>>>>> So one can proceed as follows: > >>>>>>>> > >>>>>>>> static VLC vlcs[NUM]; > >>>>>>>> static VLC_TYPE vlc_table[BUF_SIZE][2]; > >>>>>>>> > >>>>>>>> for (int i = 0, offset = 0; i < NUM; i++) { > >>>>>>>> vlcs[i].table = &vlc_table[offset]; > >>>>>>>> vlcs[i].table_allocated = BUF_SIZE - offset; > >>>>>>>> init_vlc(); > >>>>>>>> offset += vlcs[i].table_size; > >>>>>>>> } > >>>>>>>> > >>>>>>>> Of course, BUF_SIZE should be equal to the number of entries > >> actually > >>>>>>>> needed. > >>>>>>>> > >>>>>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > >>>>>>>> --- > >>>>>>>> libavcodec/bitstream.c | 3 --- > >>>>>>>> 1 file changed, 3 deletions(-) > >>>>>>>> > >>>>>>>> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c > >>>>>>>> index 03d39ad88c..4ffec7e17a 100644 > >>>>>>>> --- a/libavcodec/bitstream.c > >>>>>>>> +++ b/libavcodec/bitstream.c > >>>>>>>> @@ -281,9 +281,6 @@ static int vlc_common_end(VLC *vlc, int > nb_bits, > >>>> int > >>>>>>>> nb_codes, VLCcode *codes, > >>>>>>>> int ret = build_table(vlc, nb_bits, nb_codes, codes, flags); > >>>>>>>> > >>>>>>>> if (flags & INIT_VLC_USE_NEW_STATIC) { > >>>>>>>> - if(vlc->table_size != vlc->table_allocated) > >>>>>>>> - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", > >>>>>>>> vlc->table_size, vlc->table_allocated); > >>>>>>>> - > >>>>>>>> av_assert0(ret >= 0); > >>>>>>>> *vlc_arg = *vlc; > >>>>>>>> } else { > >>>>>>>> -- > >>>>>>>> 2.25.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".
Andreas Rheinhardt: > Paul B Mahol: >> Than how one is supposed to guess correct size to put when this log is gone? >> > The developer who wants to make VLCs static just has to use a huge array > (in order not to run into the abort) and get the value of the final > offset parameter (either via av_log or via a debugger); or you just sum > the VLC.table_size values before you make the VLCs static. It is no big > deal and it is not much different from what it is now. > I forgot to mention that this last statement only applies to finding the total size of the buffer. But as long as this log statement is there, this is not enough: One really needs all the sizes (or offsets as one prefers) which is a bit more work (and leads to some unnecessary code and unnecessary tables). >> On Tue, Nov 10, 2020 at 1:15 PM Andreas Rheinhardt < >> andreas.rheinhardt@gmail.com> wrote: >> >>> Paul B Mahol: >>>> On Tue, Nov 10, 2020 at 12:31 PM Andreas Rheinhardt < >>>> andreas.rheinhardt@gmail.com> wrote: >>>> >>>>> Paul B Mahol: >>>>>> Because it is a bad idea. >>>>> >>>>> And I still like to hear a reason for this. >>>>> >>>>> >>>> Code is there so correct intended size is always allocated. >>>> >>> >>> No. There are two ways to create a VLC: With the INIT_VLC_USE_NEW_STATIC >>> flag set and without it. >>> If it is unset, ff_init_vlc_sparse() (and ff_init_vlc_from_lengths(), >>> too) allocates the needed space for the VLC table (actually, it >>> overallocates) and the VLC must in turn be freed via ff_free_vlc(). In >>> this case the whole given VLC is treated as uninitialized, i.e. >>> vlc->table_allocated is ignored. >>> >>> If it is set, then vlc->table must point to an array VLC_TYPE[][2] with >>> at least vlc->table_allocated elements and vlc->table_allocated must be >>> sufficient for the whole VLC: This buffer will never be (re)allocated; >>> it actually points to static storage and not to anything that is >>> heap-allocated. Given the error message that this commit intends to >>> remove there is currently a requirement for table_allocated to be >>> exactly equal to the actually required size. And this requires tables of >>> the sizes if VLCs are initialized in a loop (alternatively one can of >>> course unroll the loop and hardcode the values via one of the INIT_VLC >>> macros). >>> >>>> >>>>>> No need to change code that worked for ages. >>>>>> >>>>> >>>>> It allows to remove useless tables and it simplifies making more VLCs >>>>> that should be static static. >>>>> >>>> >>>> What is static static? How does it allows to remove useless tables? >>>> >>> >>> And this requirement is just a pointless straitjacket. If one drops it, >>> one can remove the useless tables by using the pattern mentioned in the >>> commit message. Several commits of this series provide examples for >>> this; e.g. #12 >>> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272139.html), >>> #40 >>> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272167.html), >>> #53 >>> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272179.html), >>> #57 >>> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272183.html) or >>> #88 (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272215.html). >>> >>> These are only examples where existing offsets tables were removed; >>> making VLCs static like in #68 >>> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272195.html; >>> making these VLCs static is especially beneficial for a multithreaded >>> decoder like this one so that the VLCs can be shared between all >>> threads) would be more complicated if the requirement of always using >>> exactly the required size were not dropped. >>> >>>> >>>>> >>>>>> On Tue, Nov 10, 2020 at 12:26 PM Andreas Rheinhardt < >>>>>> andreas.rheinhardt@gmail.com> wrote: >>>>>> >>>>>>> Paul B Mahol: >>>>>>>> I do not think this is a good direction. >>>>>>>> >>>>>>> >>>>>>> Why? >>>>>>> >>>>>>>> On Tue, Nov 10, 2020 at 11:50 AM Andreas Rheinhardt < >>>>>>>> andreas.rheinhardt@gmail.com> wrote: >>>>>>>> >>>>>>>>> Right now the allocated size of the VLC table of a static VLC has to >>>>>>>>> exactly match the size actually used for the VLC: If it is not >>> enough, >>>>>>>>> abort is called; if it is more than enough, an error message is >>>>>>>>> emitted. This is no problem when one wants to initialize an >>> individual >>>>>>>>> VLC via one of the INIT_VLC macros as one just hardcodes the needed >>>>>>>>> size. Yet it is an obstacle when one wants to initialize several >>> VLCs >>>>>>>>> in a loop as one then needs to add an array for the sizes/offsets of >>>>>>>>> the VLC tables (unless max_depth of all arrays is one in which case >>>>>>>>> the sizes are derivable from the number of bits used). >>>>>>>>> >>>>>>>>> Yet said size array is not necessary if one removes the warning for >>>>> too >>>>>>>>> big buffers. The reason is that the amount of entries needed for the >>>>>>>>> table is of course generated as a byproduct of initializing the VLC. >>>>>>>>> So one can proceed as follows: >>>>>>>>> >>>>>>>>> static VLC vlcs[NUM]; >>>>>>>>> static VLC_TYPE vlc_table[BUF_SIZE][2]; >>>>>>>>> >>>>>>>>> for (int i = 0, offset = 0; i < NUM; i++) { >>>>>>>>> vlcs[i].table = &vlc_table[offset]; >>>>>>>>> vlcs[i].table_allocated = BUF_SIZE - offset; >>>>>>>>> init_vlc(); >>>>>>>>> offset += vlcs[i].table_size; >>>>>>>>> } >>>>>>>>> >>>>>>>>> Of course, BUF_SIZE should be equal to the number of entries >>> actually >>>>>>>>> needed. >>>>>>>>> >>>>>>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >>>>>>>>> --- >>>>>>>>> libavcodec/bitstream.c | 3 --- >>>>>>>>> 1 file changed, 3 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c >>>>>>>>> index 03d39ad88c..4ffec7e17a 100644 >>>>>>>>> --- a/libavcodec/bitstream.c >>>>>>>>> +++ b/libavcodec/bitstream.c >>>>>>>>> @@ -281,9 +281,6 @@ static int vlc_common_end(VLC *vlc, int nb_bits, >>>>> int >>>>>>>>> nb_codes, VLCcode *codes, >>>>>>>>> int ret = build_table(vlc, nb_bits, nb_codes, codes, flags); >>>>>>>>> >>>>>>>>> if (flags & INIT_VLC_USE_NEW_STATIC) { >>>>>>>>> - if(vlc->table_size != vlc->table_allocated) >>>>>>>>> - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", >>>>>>>>> vlc->table_size, vlc->table_allocated); >>>>>>>>> - >>>>>>>>> av_assert0(ret >= 0); >>>>>>>>> *vlc_arg = *vlc; >>>>>>>>> } else { >>>>>>>>> -- >>>>>>>>> 2.25.1 >>>>>>>>> >>>>>>> >>>>>
On Tue, Nov 10, 2020 at 01:37:26PM +0100, Paul B Mahol wrote:
> Big disagree, please keep this log intact.
i agree, for people writing alot of codecs like paul is doing this av_log
is helpful
Thanks
[...]
On Tue, Nov 10, 2020 at 5:35 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Tue, Nov 10, 2020 at 01:37:26PM +0100, Paul B Mahol wrote: > > Big disagree, please keep this log intact. > > i agree, for people writing alot of codecs like paul is doing this av_log > is helpful > > It can be made less verbose, e.g. into debug. But I see no point in removing it completely. > Thanks > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > it is not once nor twice but times without number that the same ideas make > their appearance in the world. -- Aristotle > _______________________________________________ > 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".
Paul B Mahol: > On Tue, Nov 10, 2020 at 5:35 PM Michael Niedermayer <michael@niedermayer.cc> > wrote: > >> On Tue, Nov 10, 2020 at 01:37:26PM +0100, Paul B Mahol wrote: >>> Big disagree, please keep this log intact. >> >> i agree, for people writing alot of codecs like paul is doing this av_log >> is helpful >> >> It can be made less verbose, e.g. into debug. > But I see no point in removing it completely. > Another alternative would be to add a flag that indicates that it is expected that allocated_size is bigger than the needed size. - Andreas
diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c index 03d39ad88c..4ffec7e17a 100644 --- a/libavcodec/bitstream.c +++ b/libavcodec/bitstream.c @@ -281,9 +281,6 @@ static int vlc_common_end(VLC *vlc, int nb_bits, int nb_codes, VLCcode *codes, int ret = build_table(vlc, nb_bits, nb_codes, codes, flags); if (flags & INIT_VLC_USE_NEW_STATIC) { - if(vlc->table_size != vlc->table_allocated) - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", vlc->table_size, vlc->table_allocated); - av_assert0(ret >= 0); *vlc_arg = *vlc; } else {
Right now the allocated size of the VLC table of a static VLC has to exactly match the size actually used for the VLC: If it is not enough, abort is called; if it is more than enough, an error message is emitted. This is no problem when one wants to initialize an individual VLC via one of the INIT_VLC macros as one just hardcodes the needed size. Yet it is an obstacle when one wants to initialize several VLCs in a loop as one then needs to add an array for the sizes/offsets of the VLC tables (unless max_depth of all arrays is one in which case the sizes are derivable from the number of bits used). Yet said size array is not necessary if one removes the warning for too big buffers. The reason is that the amount of entries needed for the table is of course generated as a byproduct of initializing the VLC. So one can proceed as follows: static VLC vlcs[NUM]; static VLC_TYPE vlc_table[BUF_SIZE][2]; for (int i = 0, offset = 0; i < NUM; i++) { vlcs[i].table = &vlc_table[offset]; vlcs[i].table_allocated = BUF_SIZE - offset; init_vlc(); offset += vlcs[i].table_size; } Of course, BUF_SIZE should be equal to the number of entries actually needed. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavcodec/bitstream.c | 3 --- 1 file changed, 3 deletions(-)