Paul B Mahol:
> On Wed, Jun 28, 2023 at 12:26 AM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>> On Tue, Jun 27, 2023 at 11:45 PM Andreas Rheinhardt <
>>> andreas.rheinha...@outlook.com> wrote:
>>>
>>>> Paul B Mahol:
>>>>> On Tue, Jun 27, 2023 at 11:05 PM Andreas Rheinhardt <
>>>>> andreas.rheinha...@outlook.com> wrote:
>>>>>
>>>>>> Paul B Mahol:
>>>>>>> On Tue, Jun 27, 2023 at 9:47 PM Andreas Rheinhardt <
>>>>>>> andreas.rheinha...@outlook.com> wrote:
>>>>>>>
>>>>>>>> Paul B Mahol:
>>>>>>>>> Patch attached.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Where do you intend to use this? What is the point of it?
>>>>>>>> After all, using this value in GET_VLC makes no sense; only
>>>> compile-time
>>>>>>>> constants do.
>>>>>>>>
>>>>>>>
>>>>>>> It works when used in ac-4 as get_vlc2.
>>>>>>>
>>>>>>
>>>>>> Could you please define "works"? Using a non-compile-time constant
>> will
>>>>>> not benefit at all; it will only lead to more runtime checks.
>>>>>>
>>>>>
>>>>> I do not follow your worries.
>>>>> I can not use compile time constant as its very complicated code.
>>>>>
>>>>
>>>> Let's take a look at GET_VLC:
>>>> #define GET_VLC(code, name, gb, table, bits, max_depth) \
>>>> do { \
>>>> int n, nb_bits; \
>>>> unsigned int index; \
>>>> \
>>>> index = SHOW_UBITS(name, gb, bits); \
>>>> code = table[index].sym; \
>>>> n = table[index].len; \
>>>> \
>>>> if (max_depth > 1 && n < 0) { \
>>>> LAST_SKIP_BITS(name, gb, bits); \
>>>> UPDATE_CACHE(name, gb); \
>>>> \
>>>> nb_bits = -n; \
>>>> \
>>>> index = SHOW_UBITS(name, gb, nb_bits) + code; \
>>>> code = table[index].sym; \
>>>> n = table[index].len; \
>>>> if (max_depth > 2 && n < 0) { \
>>>> LAST_SKIP_BITS(name, gb, nb_bits); \
>>>> UPDATE_CACHE(name, gb); \
>>>> \
>>>> nb_bits = -n; \
>>>> \
>>>> index = SHOW_UBITS(name, gb, nb_bits) + code; \
>>>> code = table[index].sym; \
>>>> n = table[index].len; \
>>>> } \
>>>> } \
>>>> SKIP_BITS(name, gb, n); \
>>>> } while (0)
>>>>
>>>> If max_depth is not a compile-time constant, then the compiler will have
>>>> to perform both of the max_depth > 1 && n < 0 checks; yet, this is not
>>>> useful: If the depth of a particular VLC is (say) 1, then none of the
>>>> possible bits read will lead to reloading at all, because the n < 0
>>>> condition will never be true; the only reason this condition exists is
>>>> to use a compile-time upper bound, so that one can eliminate the reload
>>>> code (and in particular, avoid the runtime checks).
>>>>
>>>>> Works means that vlc code is extracted correctly.
>>>>>
>>>>
>>>> If you have no upper bound about max_depth and it works, then use 3.
>>>>
>>>
>>> It does not work to use 3 all the time. And that one never worked in any
>>> codec so far.
>>>
>>
>> I just ran FATE with the check for max_depth removed from GET_VLC and
>> from read_vlc for the cached API (effectively setting max_depth to 3
>> everywhere). It passed. So it "works" (but in a suboptimal way). At
>> least it does if you have ordinary VLCs (created by the vlc.c
>> functions). Are you doing anything special with them or so?
>>
>
> FATE code coverage is very limited.
>
> Also I do not follow your reasoning about this added field at all.
>
> What is calculated over and over again in each get_vlc2() call?
>
Nothing is calculated over and over again in each get_vlc2() call; but
if you use a non-compile-time constant, then the check for max_depth is
performed in each get_vlc2() call, even though it is unnecessary.
- 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-requ...@ffmpeg.org with subject "unsubscribe".
