[FFmpeg-devel] [PATCH 3/3] lavc/libopenjpegdec: support interlaced layout

[Tim Nicholson]

On 19/02/13 10:19, Matthieu Bouron wrote:
> On Mon, Feb 18, 2013 at 08:20:53AM +0000, Tim Nicholson wrote:
>> On 16/02/13 12:52, Matthieu Bouron wrote:
>>> Fixes tickets #1102.
>>> ---
>>>  libavcodec/libopenjpegdec.c | 61 +++++++++++++++++++++++++++++++++------------
>>>  1 file changed, 45 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c
>>> index eeb3dd6..ad7866c 100644
>>> --- a/libavcodec/libopenjpegdec.c
>>> +++ b/libavcodec/libopenjpegdec.c
>>> @@ -69,6 +69,7 @@ typedef struct {
>>>      opj_dparameters_t dec_params;
>>>      AVFrame image;
>>>      int lowqual;
>>> +    int curpict;
>>>  } LibOpenJPEGContext;
>>>  
>>>  static inline int libopenjpeg_matches_pix_fmt(const opj_image_t *image, enum AVPixelFormat pix_fmt)
>>> @@ -149,12 +150,13 @@ static inline int libopenjpeg_ispacked(enum AVPixelFormat pix_fmt)
>>>      return 1;
>>>  }
>>>  
>>> -static inline void libopenjpeg_copy_to_packed8(AVFrame *picture, opj_image_t *image) {
>>> +static inline void libopenjpeg_copy_to_packed8(AVFrame *picture, opj_image_t *image, int field) {
>>>      uint8_t *img_ptr;
>>> -    int index, x, y, c;
>>> +    int index, x, y, c, line;
>>>      for (y = 0; y < picture->height; y++) {
>>>          index = y*picture->width;
>>> -        img_ptr = picture->data[0] + y*picture->linesize[0];
>>> +        line = picture->interlaced_frame * (field + y) + y;
>>> +        img_ptr = picture->data[0] + line*picture->linesize[0];
>>>          for (x = 0; x < picture->width; x++, index++) {
>>>              for (c = 0; c < image->numcomps; c++) {
>>>                  *img_ptr++ = image->comps[c].data[index];
>>> @@ -163,16 +165,17 @@ static inline void libopenjpeg_copy_to_packed8(AVFrame *picture, opj_image_t *im
>>>      }
>>>  }
>>>  
>>> -static inline void libopenjpeg_copy_to_packed16(AVFrame *picture, opj_image_t *image) {
>>> +static inline void libopenjpeg_copy_to_packed16(AVFrame *picture, opj_image_t *image, int field) {
>>>      uint16_t *img_ptr;
>>> -    int index, x, y, c;
>>> +    int index, x, y, c, line;
>>>      int adjust[4];
>>>      for (x = 0; x < image->numcomps; x++)
>>>          adjust[x] = FFMAX(FFMIN(16 - image->comps[x].prec, 8), 0);
>>>  
>>>      for (y = 0; y < picture->height; y++) {
>>>          index = y*picture->width;
>>> -        img_ptr = (uint16_t*) (picture->data[0] + y*picture->linesize[0]);
>>> +        line = picture->interlaced_frame * (field + y) + y;
>>> +        img_ptr = (uint16_t*) (picture->data[0] + line*picture->linesize[0]);
>>>          for (x = 0; x < picture->width; x++, index++) {
>>>              for (c = 0; c < image->numcomps; c++) {
>>>                  *img_ptr++ = image->comps[c].data[index] << adjust[c];
>>> @@ -181,7 +184,7 @@ static inline void libopenjpeg_copy_to_packed16(AVFrame *picture, opj_image_t *i
>>>      }
>>>  }
>>>  
>>> -static inline void libopenjpeg_copyto8(AVFrame *picture, opj_image_t *image) {
>>> +static inline void libopenjpeg_copyto8(AVFrame *picture, opj_image_t *image, int field) {
>>>      int *comp_data;
>>>      uint8_t *img_ptr;
>>>      int index, x, y;
>>> @@ -189,7 +192,8 @@ static inline void libopenjpeg_copyto8(AVFrame *picture, opj_image_t *image) {
>>>      for (index = 0; index < image->numcomps; index++) {
>>>          comp_data = image->comps[index].data;
>>>          for (y = 0; y < image->comps[index].h; y++) {
>>> -            img_ptr = picture->data[index] + y * picture->linesize[index];
>>> +            int line = picture->interlaced_frame * (field + y) + y;
>>> +            img_ptr = picture->data[index] + line * picture->linesize[index];
>>>              for (x = 0; x < image->comps[index].w; x++) {
>>>                  *img_ptr = (uint8_t) *comp_data;
>>>                  img_ptr++;
>>> @@ -199,14 +203,15 @@ static inline void libopenjpeg_copyto8(AVFrame *picture, opj_image_t *image) {
>>>      }
>>>  }
>>>  
>>> -static inline void libopenjpeg_copyto16(AVFrame *picture, opj_image_t *image) {
>>> +static inline void libopenjpeg_copyto16(AVFrame *picture, opj_image_t *image, int field) {
>>>      int *comp_data;
>>>      uint16_t *img_ptr;
>>>      int index, x, y;
>>>      for (index = 0; index < image->numcomps; index++) {
>>>          comp_data = image->comps[index].data;
>>>          for (y = 0; y < image->comps[index].h; y++) {
>>> -            img_ptr = (uint16_t*) (picture->data[index] + y * picture->linesize[index]);
>>> +            int line = picture->interlaced_frame * (field + y) + y;
>>> +            img_ptr = (uint16_t*) (picture->data[index] + line * picture->linesize[index]);
>>>              for (x = 0; x < image->comps[index].w; x++) {
>>>                  *img_ptr = *comp_data;
>>>                  img_ptr++;
>>> @@ -249,9 +254,18 @@ static int libopenjpeg_decode_frame(AVCodecContext *avctx,
>>>      int width, height, ret = -1;
>>>      int pixel_size = 0;
>>>      int ispacked = 0;
>>> +    int field;
>>>      int i;
>>>  
>>>      *got_frame = 0;
>>> +    picture->interlaced_frame = avctx->field_order > AV_FIELD_PROGRESSIVE;
>>> +    picture->top_field_first  = avctx->field_order == AV_FIELD_TT;
>>> +    field = ctx->curpict ^ !picture->top_field_first;
>>> +
>>
>> Here an elsewhere. AV_FIELD_TT is used for top field first. I don't know
>> about the specifics for j2k, but usually* the correct value is
>> AV_FIELD_TB. This maps to the "normal" top field first flag (0x09) in
>> mov.c/movenc.c. It is counter intuitive, and the comments against the
>> values in avcodec.h are worded the reverse of the way it is written in
>> qtff, which itself is less than clear as to the meaning, hence my query.
> 
> Should this behaviour in mov/movenc be corrected to match the correct
> AVFieldOrder ?
> 

I think mov/movenc is correct. Normal (rawvideo, prores etc) tff
material ends up with 0x09 which is the expected value.

>>
>> Can yo confirm that a j2k rewrapped in a mov wrapper ends up with the
>> correct value with this patch?
>>
> 
> A j2k rewrapped with this patch ends up with a fiel atom with value =
> 0x01 (AV_FIELD_TT).
> 

Yes, but is this the correct value? AFAIK only mjpeg uses this, all
other codecs use 0x09.

Of course if ffmpeg behaved itself, it wouldn't write the fiel atom for
this codec as it isnt necessary, but it worries me if we write a value
the reverse of reality. Maybe the safe thing is to suppress it for this
codec as an extension to Michael's recent hotfix.

> I used AV_FIELD_TT and AV_FIELD_BB as it felt natural to me and
> unfortunately S422M does not seem to specify this.

No, its very much an Apple thing and badly explained in their docs, and
I agree TT and BB feel natural, but looking at a wide selection of files
reveals that TB and BT are much more common, except as I say, for mjepg.

> 
> Have you any samples of this kind of files ? It would help to find out if
> j2k in MXF is TB/BT or TT/BB or maybe someone can confirm the coding
> order ?
> 

All my sample j2k in mov files don't have the fiel atom at all....

> Regards,
> Matthieu
>[..]


-- 
Tim