Re: [FFmpeg-devel] [PATCH] avcodec/cbs_jpeg: Fix size of huffman symbol table array

From: Mark Thompson <sw@jkqxz.net>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH] avcodec/cbs_jpeg: Fix size of huffman symbol table array
Date: Sat, 30 Apr 2022 20:18:18 +0100
Message-ID: <f05ad5ab-82ff-c58d-0af4-2de93e485d6f@jkqxz.net> (raw)
In-Reply-To: <AS8PR01MB794410B616618177E294BEFD8FFF9@AS8PR01MB7944.eurprd01.prod.exchangelabs.com>

On 30/04/2022 19:38, Andreas Rheinhardt wrote:
> Mark Thompson:
>> On 08/02/2022 09:41, Andreas Rheinhardt wrote:
>>> L[i] can be in the range of 0-255, see table B.5 of ITU T.81.
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>>> ---
>>>    libavcodec/cbs_jpeg.h                 | 2 +-
>>>    libavcodec/cbs_jpeg_syntax_template.c | 4 ++--
>>>    2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> Do you have a valid file showing this?  Not all values are allowed.
>>
> 
> Where is this said in the spec?
> The file jpg/12bpp.jpg from the FATE-suite triggers this. It has a
> Huffman table with 226 entries.
> (Sorry, should have mentioned the sample in the commit message.)
> 
>> I guess I must have written it, but I have no idea where 224 came from.
>> As far as I know the worst case is in AC tables: 10 category values * 16
>> run lengths + 2 special cases = 162 (which could indeed all be dumped in
>> the same code length if you want to be pathological).
> 
> I have never heard of these restrictions. Would you care to elaborate
> which part of the spec they refer to?

Urgh.  I was thinking of F.1.2.2.1, defining 10 categories (figure F.1 illustrates the 162 possible values).

F.1.5.2 for 12-bit extends that with four additional categories for a total of 226 values.  Maybe that's where 224 came from, except typoed.

> Anyway, IIRC there is no restriction against duplicates in the Huffman
> table, so one could use even more than 256 values (i.e. there might be
> spec-compliant pictures that are not supported by both our decoder and
> the current version of cbs_jpeg); it just makes no sense. Notice that
> the sample mentioned above has no duplicate values in any Huffman table.

If duplicates were allowed then the whole thing could have a lot more than 256 entries (e.g. 255 entries in each of 9-16 bit length (covering ~half the remaining space in each case) is 2040).  I feel like there must be a prohibition against this somewhere, though I don't see it.

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