[FFmpeg-devel] [PATCH] libopenjpeg J2K encoder

[Michael Bradshaw]

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!

Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Added-support-for-J2K-encoding-with-libopenjpeg.patch
Type: application/octet-stream
Size: 16153 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111117/b7cb3a8e/attachment.obj>