| Message ID | f86bb70e-139e-32db-2ff9-1133ca4dc92b@googlemail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Oct 11, 2016 at 9:57 AM, Andreas Cadhalpun < andreas.cadhalpun@googlemail.com> wrote: > openjpeg 2 sets the data pointers of the image components to NULL, > causing segfaults if the image is reused. > I've never seen this issue. Is there a particular version of OpenJPEG or reproduction steps available? Where are the data pointers being set to NULL here? Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > --- > > The relevant openjpeg2 code is: > https://sources.debian.net/src/openjpeg2/2.1.2-1/src/lib/ > openjp2/j2k.c/?hl=10253#L10247 > > --- > libavcodec/libopenjpegenc.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/libavcodec/libopenjpegenc.c b/libavcodec/libopenjpegenc.c > index 023fdf4..2f0ab20 100644 > --- a/libavcodec/libopenjpegenc.c > +++ b/libavcodec/libopenjpegenc.c > @@ -776,6 +776,16 @@ static int libopenjpeg_encode_frame(AVCodecContext > *avctx, AVPacket *pkt, > goto done; > } > > + // openjpeg 2 sets the data pointers of the image components to NULL. > + // Thus the image can't be reused. > + opj_image_destroy(ctx->image); > + ctx->image = mj2_create_image(avctx, &ctx->enc_params); > + if (!ctx->image) { > + av_log(avctx, AV_LOG_ERROR, "Error creating the mj2 image\n"); > + ret = AVERROR(EINVAL); > + goto done; > + } > + > av_shrink_packet(pkt, writer.pos); > #endif // OPENJPEG_MAJOR_VERSION == 1 > > -- > 2.9.3 > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 12.10.2016 03:42, Michael Bradshaw wrote: > On Tue, Oct 11, 2016 at 9:57 AM, Andreas Cadhalpun < > andreas.cadhalpun@googlemail.com> wrote: > >> openjpeg 2 sets the data pointers of the image components to NULL, >> causing segfaults if the image is reused. >> > > I've never seen this issue. That's strange, as it happens practically always here (on Debian testing/sid). > Is there a particular version of OpenJPEG or The OpenJPEG version is 2.1.2, i.e. the latest. > reproduction steps available? For example: $ ffmpeg -f lavfi -i testsrc -c:v libopenjpeg -f null /dev/null > Where are the data pointers being set to NULL here? >> >> The relevant openjpeg2 code is: >> https://sources.debian.net/src/openjpeg2/2.1.2-1/src/lib/openjp2/j2k.c/?hl=10253#L10247 I didn't include this link in the commit message, as it will become invalid, as soon as the next OpenJPEG version enters Debian. For future reference, the code is: /* TODO_MSD: Find a better way */ if (p_image->comps) { OPJ_UINT32 it_comp; for (it_comp = 0 ; it_comp < p_image->numcomps; it_comp++) { if (p_image->comps[it_comp].data) { p_j2k->m_private_image->comps[it_comp].data =p_image->comps[it_comp].data; p_image->comps[it_comp].data = NULL; } } } Best regards, Andreas
On Wed, Oct 12, 2016 at 8:45 AM, Andreas Cadhalpun < andreas.cadhalpun@googlemail.com> wrote: > On 12.10.2016 03:42, Michael Bradshaw wrote: > > On Tue, Oct 11, 2016 at 9:57 AM, Andreas Cadhalpun < > > andreas.cadhalpun@googlemail.com> wrote: > > > >> openjpeg 2 sets the data pointers of the image components to NULL, > >> causing segfaults if the image is reused. > >> > > > > I've never seen this issue. > > That's strange, as it happens practically always here (on Debian > testing/sid). > > > Is there a particular version of OpenJPEG or > > The OpenJPEG version is 2.1.2, i.e. the latest. > > > reproduction steps available? > > For example: > $ ffmpeg -f lavfi -i testsrc -c:v libopenjpeg -f null /dev/null Thanks for that (and the link back to the OpenJPEG source). Well dang. I think it would be better to change the patch to completely remove the image member from LibOpenJPEGContext, and instead just create a local image (and destroy it) for every call to libopenjpeg_encode_frame.
diff --git a/libavcodec/libopenjpegenc.c b/libavcodec/libopenjpegenc.c index 023fdf4..2f0ab20 100644 --- a/libavcodec/libopenjpegenc.c +++ b/libavcodec/libopenjpegenc.c @@ -776,6 +776,16 @@ static int libopenjpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt, goto done; } + // openjpeg 2 sets the data pointers of the image components to NULL. + // Thus the image can't be reused. + opj_image_destroy(ctx->image); + ctx->image = mj2_create_image(avctx, &ctx->enc_params); + if (!ctx->image) { + av_log(avctx, AV_LOG_ERROR, "Error creating the mj2 image\n"); + ret = AVERROR(EINVAL); + goto done; + } + av_shrink_packet(pkt, writer.pos); #endif // OPENJPEG_MAJOR_VERSION == 1
openjpeg 2 sets the data pointers of the image components to NULL, causing segfaults if the image is reused. Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> --- The relevant openjpeg2 code is: https://sources.debian.net/src/openjpeg2/2.1.2-1/src/lib/openjp2/j2k.c/?hl=10253#L10247 --- libavcodec/libopenjpegenc.c | 10 ++++++++++ 1 file changed, 10 insertions(+)