[FFmpeg-devel] [PATCH] libopenjpeg J2K encoder

[Michael Niedermayer]

On Thu, Nov 17, 2011 at 05:00:52PM -0700, Michael Bradshaw wrote:
> On Thu, Nov 17, 2011 at 4:08 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Thu, Nov 17, 2011 at 11:56:29PM +0100, Michael Niedermayer wrote:
> >> On Thu, Nov 17, 2011 at 11:42:26PM +0100, Michael Niedermayer wrote:
> >> > Hi
> >> >
> >> > On Thu, Nov 17, 2011 at 03:18:34PM -0700, Michael Bradshaw wrote:
> >> > > > Michael Bradshaw <mbradshaw <at> sorensonmedia.com> writes:
> >> [...]
> >> > > Attached is the revised patch.
> >> >
> >> > Ill look at it in a moment, thanks
> >>
> >> same gdb crash and under valgrind i get:
> >>
> >> Conditional jump or move depends on uninitialised value(s)
> >>    at 0x6A01E69: opj_event_msg (in /usr/lib/libopenjpeg-2.1.3.0.so)
> >>    by 0x6A05F7D: j2k_encode (in /usr/lib/libopenjpeg-2.1.3.0.so)
> >>    by 0x786496: libopenjpeg_encode_frame (libopenjpegenc.c:256)
> >>    by 0x8A40AE: avcodec_encode_video (utils.c:747)
> >>    by 0x43F855: output_packet (ffmpeg.c:1324)
> >>    by 0x443C7E: transcode (ffmpeg.c:2710)
> >>    by 0x447A55: main (ffmpeg.c:4758)
> >>
> >> (i get more stuff from valgrind but the others look unrelated to me)
> >>
> >> but otherwise i get a good looking video from valgrinded ffmpeg_g
> >
> > following fixes the crash for me:
> > @@ -34,6 +34,7 @@ typedef struct {
> >     opj_image_t *image;
> >     opj_cparameters_t enc_params;
> >     opj_cinfo_t *compress;
> > +    opj_event_mgr_t event_mgr;
> >  } LibOpenJPEGContext;
> >
> >  static opj_image_t *mj2_create_image(AVCodecContext *avctx, opj_cparameters_t *parameters)
> > @@ -160,6 +161,8 @@ static av_cold int libopenjpeg_encode_init(AVCodecContext *avctx)
> >         return AVERROR(EINVAL);
> >     }
> >
> > +    opj_set_event_mgr(ctx->compress, &ctx->event_mgr, 0);
> > +
> >     return 0;
> >  }
> 
> You beat me to it.  Thanks for the quick fix.  I've incorporated it
> (as well as created two functions for the event manager to get
> feedback from libopenjpeg).  I've also changed the way the copy
> functions iterate over the buffers so that if linesize != image width
> the data will still get properly copied.  My 300x800 video test was
> getting mangled.
> 
> I've also incremented LIBAVCODEC_VERSION_MINOR, added a line to the
> Changelog, and found that hiding white space.  See attached for the
> full patch.
> 
> About the pixel format layout for identifying YCbCr vs RGB, I think
> that's something may be useful to those patching the decoder (see
> http://ffmpeg.org/pipermail/ffmpeg-devel/2011-November/116649.html).
> 
> Let me know if more changes are needed.  Thanks for all your help in
> this review process as well!

patch applied, thanks!

To maintain the code, best is to have your own public git clone of
ffmpeg (for example on github). Where you can freely work and
integrate patches from others that you reviewed, ...,
And once you are happy with whats in your repo you tell me to merge it
and ill merge it then into main ffmpeg.

Ill post a patch or 2 in a moment that you can review/apply if you like

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111118/0f327f2c/attachment.asc>