Am Do., 18. Juni 2020 um 21:50 Uhr schrieb <gautamr...@gmail.com>:
>
> From: Gautam Ramakrishnan <gautamr...@gmail.com>
>
> This is with reference to my previous email on the mailing list
> with subject: "query on pixel formats".
> I wish to cleanup some errors in the decoder code. These changes
> would allow the samples p1_01.j2k and p1_07.j2k to be decoded.
> However, I am facing issues with pixel format selection and have
> currently forced the pixel formats to demonstrate the changes made.
> Would be grateful if anyone could suggest modifications to the pix
> format selection.
The patch looks to me as if it should be split but maybe I
misunderstand and the changes are strongly related?
> ---
> libavcodec/jpeg2000.c | 3 ---
> libavcodec/jpeg2000dec.c | 53 ++++++++++++++++++++++++++--------------
> 2 files changed, 34 insertions(+), 22 deletions(-)
>
> diff --git a/libavcodec/jpeg2000.c b/libavcodec/jpeg2000.c
> index 73206d17f3..1aca31ffa4 100644
> --- a/libavcodec/jpeg2000.c
> +++ b/libavcodec/jpeg2000.c
> @@ -509,9 +509,6 @@ int ff_jpeg2000_init_component(Jpeg2000Component *comp,
> // update precincts size: 2^n value
> reslevel->log2_prec_width = codsty->log2_prec_widths[reslevelno];
> reslevel->log2_prec_height = codsty->log2_prec_heights[reslevelno];
> - if (!reslevel->log2_prec_width || !reslevel->log2_prec_height) {
> - return AVERROR_INVALIDDATA;
> - }
Why exactly is this a good idea?
>
> /* Number of bands for each resolution level */
> if (reslevelno == 0)
> diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> index ab36009a2d..ae63c68ca8 100644
> --- a/libavcodec/jpeg2000dec.c
> +++ b/libavcodec/jpeg2000dec.c
> @@ -269,6 +269,7 @@ static int get_siz(Jpeg2000DecoderContext *s)
> const enum AVPixelFormat *possible_fmts = NULL;
> int possible_fmts_nb = 0;
> int ret;
> + int dimx, dimy;
>
> if (bytestream2_get_bytes_left(&s->g) < 36) {
> av_log(s->avctx, AV_LOG_ERROR, "Insufficient space for SIZ\n");
> @@ -286,10 +287,6 @@ static int get_siz(Jpeg2000DecoderContext *s)
> s->tile_offset_y = bytestream2_get_be32u(&s->g); // YT0Siz
> ncomponents = bytestream2_get_be16u(&s->g); // CSiz
> - if (s->image_offset_x || s->image_offset_y) {
> - avpriv_request_sample(s->avctx, "Support for image offsets");
> - return AVERROR_PATCHWELCOME;
> - }
I would have expected this change to be part of a patch "support
image offsets".
> if (av_image_check_size2(s->width, s->height, s->avctx->max_pixels,
> AV_PIX_FMT_NONE, 0, s->avctx)) {
> avpriv_request_sample(s->avctx, "Large Dimensions");
> return AVERROR_PATCHWELCOME;
> @@ -371,11 +368,18 @@ static int get_siz(Jpeg2000DecoderContext *s)
> }
>
> /* compute image size with reduction factor */
> - ret = ff_set_dimensions(s->avctx,
> - ff_jpeg2000_ceildivpow2(s->width - s->image_offset_x,
> - s->reduction_factor),
> - ff_jpeg2000_ceildivpow2(s->height - s->image_offset_y,
> - s->reduction_factor));
> + dimx = ff_jpeg2000_ceildivpow2(s->width - s->image_offset_x,
> + s->reduction_factor);
> + dimy = ff_jpeg2000_ceildivpow2(s->height - s->image_offset_y,
> + s->reduction_factor);
> + dimx = ff_jpeg2000_ceildiv(dimx, s->cdx[0]);
> + dimy = ff_jpeg2000_ceildiv(dimy, s->cdy[0]);
> + for (i = 1; i < s->ncomponents; i++) {
> + dimx = FFMAX(dimx, ff_jpeg2000_ceildiv(dimx, s->cdx[i]));
> + dimy = FFMAX(dimy, ff_jpeg2000_ceildiv(dimy, s->cdy[i]));
> + }
> +
> + ret = ff_set_dimensions(s->avctx, dimx, dimy);
> if (ret < 0)
> return ret;
>
> @@ -427,6 +431,13 @@ static int get_siz(Jpeg2000DecoderContext *s)
> s->cdef[3] = 3;
> i = 0;
> }
> + } else if (ncomponents == 2) {
> + s->avctx->pix_fmt = AV_PIX_FMT_YA8;
> + i = 0;
I am not convinced that this is a good idea:
Why is this not detected, what is the sample and most important:
What happens if the two components have different subsampling?
> + } else if (ncomponents == 1) {
> + s->avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> + i = 0;
> + s->cdef[0] = 0;
This may be ok but why is it not detected in the existing code?
I cannot comment on the other changes.
Carl Eugen
_______________________________________________
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".
