Paul B Mahol:
> On Wed, Jun 28, 2023 at 11:57 AM Paul B Mahol <one...@gmail.com> wrote:
>
>>
>>
>> On Wed, Jun 28, 2023 at 11:11 AM Andreas Rheinhardt <
>> andreas.rheinha...@outlook.com> wrote:
>>
>>> 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.
>>>
>>
>> So what?
>> Nothing use this yet. So it does not matter.
>>
>
> As already written, cant use compile time constant at all, as there are
> many codebooks with different size and max depth.
> And codebooks are picked by other parameters, and max depth differs even
> between same codebooks set.
max_depth only needs to be a upper bound of the actual max_depth. You do
not need the real max_depth.
> And using 3 always is not efficient and also may not work reliably all the
> time.
>
I have already told you why I believe the opposite to be true for the
first statement and why I don't understand your second statement at all.
You have not given any counterarguments to my points. Please show your code.
- 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".
