| Message ID | HE1PR0301MB21548FFB5887918B425CF0908F4E9@HE1PR0301MB2154.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Commit | 9b3c46a081a9f01559082bf7a154fc6be1e06c18 |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/jpeglsdec: Don't allocate+free JPEGLSState for every frame | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
Andreas Rheinhardt: > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavcodec/jpeglsdec.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c > index 69980eaa49..92df81600b 100644 > --- a/libavcodec/jpeglsdec.c > +++ b/libavcodec/jpeglsdec.c > @@ -45,6 +45,11 @@ > */ > //#define JLS_BROKEN > > +typedef struct JpegLSDecodeContext { > + MJpegDecodeContext mjpeg; > + JLSState state; > +} JpegLSDecodeContext; > + > /** > * Decode LSE block with initialization parameters > */ > @@ -350,7 +355,7 @@ int ff_jpegls_decode_picture(MJpegDecodeContext *s, int near, > { > int i, t = 0; > uint8_t *zero, *last, *cur; > - JLSState *state; > + JLSState *const state = &((JpegLSDecodeContext*)s)->state; > int off = 0, stride = 1, width, shift, ret = 0; > int decoded_height = 0; > > @@ -360,12 +365,8 @@ int ff_jpegls_decode_picture(MJpegDecodeContext *s, int near, > last = zero; > cur = s->picture_ptr->data[0]; > > - state = av_mallocz(sizeof(JLSState)); > - if (!state) { > - av_free(zero); > - return AVERROR(ENOMEM); > - } > /* initialize JPEG-LS state from JPEG parameters */ > + memset(state, 0, sizeof(*state)); > state->near = near; > state->bpp = (s->bits < 2) ? 2 : s->bits; > state->maxval = s->maxval; > @@ -537,7 +538,6 @@ int ff_jpegls_decode_picture(MJpegDecodeContext *s, int near, > } > > end: > - av_free(state); > av_free(zero); > > return ret; > @@ -548,7 +548,7 @@ AVCodec ff_jpegls_decoder = { > .long_name = NULL_IF_CONFIG_SMALL("JPEG-LS"), > .type = AVMEDIA_TYPE_VIDEO, > .id = AV_CODEC_ID_JPEGLS, > - .priv_data_size = sizeof(MJpegDecodeContext), > + .priv_data_size = sizeof(JpegLSDecodeContext), > .init = ff_mjpeg_decode_init, > .close = ff_mjpeg_decode_end, > .receive_frame = ff_mjpeg_receive_frame, > Will apply tomorrow unless there are objections. - Andreas
On Wed, Apr 14, 2021 at 11:21:40PM +0200, Andreas Rheinhardt wrote: > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavcodec/jpeglsdec.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) [...] > @@ -548,7 +548,7 @@ AVCodec ff_jpegls_decoder = { > .long_name = NULL_IF_CONFIG_SMALL("JPEG-LS"), > .type = AVMEDIA_TYPE_VIDEO, > .id = AV_CODEC_ID_JPEGLS, > - .priv_data_size = sizeof(MJpegDecodeContext), > + .priv_data_size = sizeof(JpegLSDecodeContext), > .init = ff_mjpeg_decode_init, > .close = ff_mjpeg_decode_end, Sorry for not spotting this before it was applied but this feels problematic a mjpeg could contain jpeg-ls in fact iam pretty sure you can mix lossless and lossy coding in a single jpeg image. Not sure if there is something in jpeg-ls that would prevent it from being part of the mix. I think its easier/more robust if the same context type would be used Thanks [...]
Michael Niedermayer: > On Wed, Apr 14, 2021 at 11:21:40PM +0200, Andreas Rheinhardt wrote: >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> libavcodec/jpeglsdec.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) > [...] >> @@ -548,7 +548,7 @@ AVCodec ff_jpegls_decoder = { >> .long_name = NULL_IF_CONFIG_SMALL("JPEG-LS"), >> .type = AVMEDIA_TYPE_VIDEO, >> .id = AV_CODEC_ID_JPEGLS, >> - .priv_data_size = sizeof(MJpegDecodeContext), >> + .priv_data_size = sizeof(JpegLSDecodeContext), >> .init = ff_mjpeg_decode_init, >> .close = ff_mjpeg_decode_end, > > Sorry for not spotting this before it was applied but this feels problematic > a mjpeg could contain jpeg-ls > in fact iam pretty sure you can mix lossless and lossy coding in a single jpeg > image. Not sure if there is something in jpeg-ls that would prevent it from > being part of the mix. > I think its easier/more robust if the same context type would be used > I am now looking at this again and there is indeed nothing that prevents other codecs using ff_mjpeg_decode_init() from calling ff_jpegls_decode_picture(). I don't know why I didn't check this earlier, sorry for that. And thanks. Will fix it. - Andreas
diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c index 69980eaa49..92df81600b 100644 --- a/libavcodec/jpeglsdec.c +++ b/libavcodec/jpeglsdec.c @@ -45,6 +45,11 @@ */ //#define JLS_BROKEN +typedef struct JpegLSDecodeContext { + MJpegDecodeContext mjpeg; + JLSState state; +} JpegLSDecodeContext; + /** * Decode LSE block with initialization parameters */ @@ -350,7 +355,7 @@ int ff_jpegls_decode_picture(MJpegDecodeContext *s, int near, { int i, t = 0; uint8_t *zero, *last, *cur; - JLSState *state; + JLSState *const state = &((JpegLSDecodeContext*)s)->state; int off = 0, stride = 1, width, shift, ret = 0; int decoded_height = 0; @@ -360,12 +365,8 @@ int ff_jpegls_decode_picture(MJpegDecodeContext *s, int near, last = zero; cur = s->picture_ptr->data[0]; - state = av_mallocz(sizeof(JLSState)); - if (!state) { - av_free(zero); - return AVERROR(ENOMEM); - } /* initialize JPEG-LS state from JPEG parameters */ + memset(state, 0, sizeof(*state)); state->near = near; state->bpp = (s->bits < 2) ? 2 : s->bits; state->maxval = s->maxval; @@ -537,7 +538,6 @@ int ff_jpegls_decode_picture(MJpegDecodeContext *s, int near, } end: - av_free(state); av_free(zero); return ret; @@ -548,7 +548,7 @@ AVCodec ff_jpegls_decoder = { .long_name = NULL_IF_CONFIG_SMALL("JPEG-LS"), .type = AVMEDIA_TYPE_VIDEO, .id = AV_CODEC_ID_JPEGLS, - .priv_data_size = sizeof(MJpegDecodeContext), + .priv_data_size = sizeof(JpegLSDecodeContext), .init = ff_mjpeg_decode_init, .close = ff_mjpeg_decode_end, .receive_frame = ff_mjpeg_receive_frame,
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/jpeglsdec.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)