[FFmpeg-devel] [RFC PATCH 4/4] libavcodec/j2kenc: Support for multiple layers

Gautam Ramakrishnan gautamramk at gmail.com
Thu Aug 20 16:33:30 EEST 2020


On Thu, Aug 20, 2020 at 12:55 AM Andreas Rheinhardt
<andreas.rheinhardt at gmail.com> wrote:
>
> Gautam Ramakrishnan:
> > On Wed, Aug 19, 2020 at 5:51 PM <gautamramk at gmail.com> wrote:
> >>
> >> From: Gautam Ramakrishnan <gautamramk at gmail.com>
> >>
> >> This patch allows setting a compression ratio and to
> >> set multiple layers. The user has to input a compression
> >> ratio for each layer.
> >> The per layer compression ration can be set as follows:
> >> -layer_rates "r1,r2,...rn"
> >> for to create 'n' layers.
> >> ---
> >>  libavcodec/j2kenc.c   | 443 ++++++++++++++++++++++++++++++++++--------
> >>  libavcodec/jpeg2000.c |  13 +-
> >>  libavcodec/jpeg2000.h |  10 +
> >>  3 files changed, 384 insertions(+), 82 deletions(-)
> >>
>
> [...]
>
> >> +static int inline check_number(char* st, int* ret) {
> >> +    int stlen = strlen(st);
> >> +    int i;
> >> +    *ret = 0;
> >> +    if (stlen <= 0) {
> >> +        return AVERROR_INVALIDDATA;
> >> +    }
> >> +    for (i = 0; i < stlen; i++) {
> >> +        if (st[i] >= '0' && st[i] <= '9') {
> >> +            *ret = (*ret) * 10 + (st[i] - '0');
> >> +        } else {
> >> +            return AVERROR_INVALIDDATA;
> >> +        }
> >> +    }
> >> +    return 0;
> >> +}
> >> +
> >> +static int parse_layer_rates(Jpeg2000EncoderContext *s)
> >> +{
> >> +    int i;
> >> +    char* token;
> >> +    int rate;
> >> +    int nlayers = 0;
> >> +    if (!s->lr_str) {
> >> +        s->nlayers = 1;
> >> +        s->layer_rates[0] = 0;
> >> +        return 0;
> >> +    }
> >> +
> >> +    token = strtok(s->lr_str, ",");
> >> +    if (!check_number(token, &rate)) {
> >> +            s->layer_rates[0] = rate <= 1 ? 0:rate;
> >> +            nlayers++;
> >> +    } else {
> >> +            return AVERROR_INVALIDDATA;
> >> +    }
> >> +
> >> +    while (1) {
> >> +        token = strtok(NULL, ",");
> >> +        if (!token)
> >> +            break;
> >> +        if (!check_number(token, &rate)) {
> >> +            s->layer_rates[nlayers] = rate <= 1 ? 0:rate;
> >> +            nlayers++;
> >> +        } else {
> >> +            return AVERROR_INVALIDDATA;
> >> +        }
> >> +    }
> >> +
> >> +    for (i = 1; i < nlayers; i++) {
> >> +        if (s->layer_rates[i] >= s->layer_rates[i-1]) {
> >> +            return AVERROR_INVALIDDATA;
> >> +        }
> >> +    }
> >> +    s->nlayers = nlayers;
> >> +    return 0;
> >> +}
> >> +
> >>  static av_cold int j2kenc_init(AVCodecContext *avctx)
> >>  {
> >>      int i, ret;
> >> @@ -1388,6 +1664,11 @@ static av_cold int j2kenc_init(AVCodecContext *avctx)
> >>
> >>      s->avctx = avctx;
> >>      av_log(s->avctx, AV_LOG_DEBUG, "init\n");
> >> +    if (parse_layer_rates(s)) {
> >> +        av_log(s, AV_LOG_WARNING, "Layer rates invalid. Shall encode with 1 layer.\n");
> >> +        s->nlayers = 1;
> >> +        s->layer_rates[0] = 0;
> >> +    }
> >>
> >>  #if FF_API_PRIVATE_OPT
> >>  FF_DISABLE_DEPRECATION_WARNINGS
> >> @@ -1408,6 +1689,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >>      memset(codsty->log2_prec_heights, 15, sizeof(codsty->log2_prec_heights));
> >>      codsty->nreslevels2decode=
> >>      codsty->nreslevels       = 7;
> >> +    codsty->nlayers          = s->nlayers;
> >>      codsty->log2_cblk_width  = 4;
> >>      codsty->log2_cblk_height = 4;
> >>      codsty->transform        = s->pred ? FF_DWT53 : FF_DWT97_INT;
> >> @@ -1489,6 +1771,7 @@ static const AVOption options[] = {
> >>      { "rpcl",          NULL,                OFFSET(prog),          AV_OPT_TYPE_CONST,   { .i64 = JPEG2000_PGOD_RPCL            }, 0,         0,           VE, "prog" },
> >>      { "pcrl",          NULL,                OFFSET(prog),          AV_OPT_TYPE_CONST,   { .i64 = JPEG2000_PGOD_PCRL            }, 0,         0,           VE, "prog" },
> >>      { "cprl",          NULL,                OFFSET(prog),          AV_OPT_TYPE_CONST,   { .i64 = JPEG2000_PGOD_CPRL            }, 0,         0,           VE, "prog" },
> >> +    { "layer_rates",   "Layer Rates",       OFFSET(lr_str),        AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, VE },
> >>      { NULL }
> >>  };
> >>
> >>
> >> --
> >> 2.17.1
> >>
> >
> > This patch seems to be breaking FATE.
> > I believe that the error is because the patch modifies the encoder
> > such that the encoded files will be slightly different now.
> > How can this be handled?
> >
> Look at the created files and see if they are ok (ideally, they should
> show an improvement). If not, modify/drop your patch; otherwise update
> the reference files by running make fate GEN=1.
>
> Just by looking at https://patchwork.ffmpeg.org/check/15622/ (i.e.
> without taking a look at the actual files and without having run fate
> with your patch myself) I can say that your patch is not ok:
>
> -8bb707e596f97451fd325dec2dd610a7 *tests/data/fate/vsynth1-jpeg2000-97.avi
> -3654620 tests/data/fate/vsynth1-jpeg2000-97.avi
> -5073771a78e1f5366a7eb0df341662fc
> *tests/data/fate/vsynth1-jpeg2000-97.out.rawvideo
> -stddev:    4.23 PSNR: 35.59 MAXDIFF:   53 bytes:  7603200/  7603200
> +5178195043ecfd671d79a194611d3573 *tests/data/fate/vsynth1-jpeg2000-97.avi
> +9986568 tests/data/fate/vsynth1-jpeg2000-97.avi
> +023623c97973489ba9cf1618b3cd25d3
> *tests/data/fate/vsynth1-jpeg2000-97.out.rawvideo
> +stddev:    3.58 PSNR: 37.04 MAXDIFF:   49 bytes:  7603200/  7603200
>
> The size of the created files increased from 3654620 to 9986568,
> although it seems to me that using multiple layers is supposed to be opt-in.
>
> Besides that: You are using an int[100] array to store the layer rates.
> Yet I don't see any check in parse_layer_rates() that actually ensures
> that there is no write beyond the end of the array. Besides that
> check_number stores the return value of strlen in an int (may be
> truncating) and does not check in the calculation.
>
> - Andreas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

Thanks a lot Andreas. Originally, the encoder used to remove some data
according to a metric called picture quality.
This picture quality is a member of avframe.
However, right now, I have removed that usage and am compressing only
based on the compression rates provided by the user.
Should I preserve the old functionality if the user does not provide a
"layer_rates"
option?


-- 
-------------
Gautam |


More information about the ffmpeg-devel mailing list