[FFmpeg-devel] [PATCH] libopenjpeg J2K encoder

[Michael Niedermayer]

On Fri, Nov 18, 2011 at 02:42:16PM -0700, Michael Bradshaw wrote:
> On Thu, Nov 17, 2011 at 6:11 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > 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
> 
> Great, thanks for the support!  I'll review those patches now.  I've
> set up a public git clone of FFmpeg (here:
> https://github.com/mjbshaw/FFmpeg-OpenJPEG-J2K-Encoder) for
> development of this encoder.

thanks
should i merge it already or do you want to do more work on it ?


[...]
> To those familiar with multithreading encoders in FFmpeg, any tips on
> what flags or functions I need to define?  Do I just define
> CODEC_CAP_FRAME_THREADS and implement init_thread_copy and
> update_thread_context functions?

We have encoders that use multithreading on slice basis. And we have
encoders that support multithreading entirely within some seperate
lib.
So if you want to implement frame based multithreading you would have
to add some support for this first to libavcodec or do it entirely in
the libopenjpeg wraper

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- 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/20111119/035d3867/attachment.asc>