[FFmpeg-devel] [PATCH] JPEG200 encoding : added option for changing default codeblock size

[Francesco, Cuzzocrea]

Hi Moritz
I check the code and it seems the error born from a previous version of 
ISO/IEC15444-1 I have
(I used a final draft not the final document and the definition was 
quite fuzzy).  Checking final
document I found that you are right.
According to this specification (Table A-18 Width or Height exponent of 
the code-blocks for the SPcod
and SPcoc parameters) code-block exponent width  xcb and height ycb are 
defined as:

xcb = value + 2   and  ycb = value+2

so xcb+ycb <=12 is the correct check. I will correct the patch.

Francesco


On 15/08/2017 23.57, Moritz Barsnick wrote:
> On Mon, Aug 07, 2017 at 10:26:21 +0200, francesco at bltitalia.com wrote:
>> Subject: [FFmpeg-devel] [PATCH] JPEG200 encoding : added option for changing default codeblock size
> Is this your commit message? It's 2000, not 200. ;-) Actually, this
> should read:
> libavcodec/j2kenc: add option for changing codeblock size
>
>> +    i = codsty->log2_cblk_width + codsty->log2_cblk_height -4;
>> +    if ( i > 12 )
>> +    {
>> +      av_log(avctx, AV_LOG_ERROR, "Invalid values for codeblocks size\n");
>> +      return -1;
>> +    }
> Bracket placement, indentation and whitespace all do not correspond to
> ffmpeg style.
>
> Apart from that: Isn't that a quite complicated way of saying
> if (codsty->log2_cblk_width + codsty->log2_cblk_height > 16)
> ??
>
>> +    { "log2_cblk_width",   "Codeblock Width",   OFFSET(codsty.log2_cblk_width),    AV_OPT_TYPE_INT,   { .i64 = 4           }, 0,     1<<10,           VE, },
>> +    { "log2_cblk_height",  "Codeblock Height",  OFFSET(codsty.log2_cblk_height),   AV_OPT_TYPE_INT,   { .i64 = 4           }, 0,     1<<10,           VE, },
> I would suggest to drop the capital letters in the option descriptions,
> but it appears to follow the style of the other options, so fine by me.
>
> And what's with the upper limits? They don't seem sane. If you choose
> either option at one of those limits, the check above will fail
> (1<<10 + 0 - 4 is waaaay larger than 12).
>
> Looking at the spec, you are mixing exponent and value. I read
> "Dimension of the code-blocks is always a power of 2 with the minimum
> height and width being 4 and and maximum height and width being 1024."
> Furthermore the sum of the exponents needs to be less than or equal to
> 12.
>
> So, the variable being log2, the limits are obviously 2 and 10, the
> default 4 (as before). And your check actually needs to read
> if (codsty->log2_cblk_width + codsty->log2_cblk_height > 12)
> .
>
> Or am I missing something?
>
>
> Moritz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
/*******************************************************************\
*    Ing. Francesco  Cuzzocrea
*    company:    BLT Italia srl        web:  http://www.blt.it
*    address:    via Rosselli, 91     city:  Lido di CAMAIORE
*    country:    ITALY                 zip:  55043-i
*      Tel. :    +39 0584 904788       Fax:  +39 0584 904789
*     e-mail:   francesco at bltitalia.com
\*******************************************************************/
-- BLT will be at IBC (15~19 September 2017) - Hall 8, Stand 8.A69 --