On Sat, 17 Jul 2021, Pierre-Anthony Lemieux wrote:

You mean something like this?

Yes, thanks.

Marton


diff --git a/libavformat/mxf.h b/libavformat/mxf.h
index ca510f5a2f..b9fe7fe7ef 100644
--- a/libavformat/mxf.h
+++ b/libavformat/mxf.h
@@ -76,7 +76,7 @@ typedef enum {
    D10D11Wrap,
    RawAWrap,
    RawVWrap,
-    AlwaysFrameWrap
+    J2KWrap
} MXFWrappingIndicatorType;

typedef struct MXFLocalTagPair {
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 7024d2ea7d..7c33150675 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -1413,8 +1413,7 @@ static void *mxf_resolve_strong_ref(MXFContext
*mxf, UID *strong_ref, enum MXFMe

static const MXFCodecUL mxf_picture_essence_container_uls[] = {
    // video essence container uls
-    { { 
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c,0x06,0x00
}, 15,   AV_CODEC_ID_JPEG2000, NULL, 16, AlwaysFrameWrap }, /*  MXF-GC
P1 Frame-Wrapped JPEG 2000 */
-    { { 
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c,0x01,0x00
}, 14,   AV_CODEC_ID_JPEG2000, NULL, 14 },
+    { { 
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c,0x01,0x00
}, 14,   AV_CODEC_ID_JPEG2000, NULL, 14, J2KWrap },
    { { 
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x10,0x60,0x01
}, 14,       AV_CODEC_ID_H264, NULL, 15 }, /* H.264 */
    { { 
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x11,0x01,0x00
}, 14,      AV_CODEC_ID_DNXHD, NULL, 14 }, /* VC-3 */
    { { 
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x12,0x01,0x00
}, 14,        AV_CODEC_ID_VC1, NULL, 14 }, /* VC-1 */
@@ -1498,8 +1497,9 @@ static MXFWrappingScheme
mxf_get_wrapping_kind(UID *essence_container_ul)
            if (val == 0x02)
                val = 0x01;
            break;
-        case AlwaysFrameWrap:
-            val = 0x01;
+        case J2KWrap:
+            if (val != 0x02)
+                val = 0x01;
            break;
    }
    if (val == 0x01)

On Sat, Jul 17, 2021 at 11:19 AM Marton Balint <c...@passwd.hu> wrote:



On Mon, 12 Jul 2021, Tomas Härdin wrote:

> sön 2021-07-11 klockan 09:47 -0700 skrev p...@sandflow.com:
>> From: Pierre-Anthony Lemieux <p...@sandflow.com>
>>
>> Signed-off-by: Pierre-Anthony Lemieux <p...@sandflow.com>
>> ---
>>
>> Notes:
>>     For JPEG 2000 essence, the MXF input format module currently uses the 
value of byte 14 of the essence container UL to determines whether the J2K essence is 
clip- (byte 14 is 0x02)
>>     or frame-wrapped (byte 14 is 0x01). This approach does work when the 
essence container UL is equal to MXFGCP1FrameWrappedPicture, in which case the 
essence is always frame-wrapped.

"This approach does "*not*" work when..."

>>
>>  libavformat/mxf.h    | 3 ++-
>>  libavformat/mxfdec.c | 4 ++++
>>  2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/mxf.h b/libavformat/mxf.h
>> index b1b1fedac7..ca510f5a2f 100644
>> --- a/libavformat/mxf.h
>> +++ b/libavformat/mxf.h
>> @@ -75,7 +75,8 @@ typedef enum {
>>      NormalWrap = 0,
>>      D10D11Wrap,
>>      RawAWrap,
>> -    RawVWrap
>> +    RawVWrap,
>> +    AlwaysFrameWrap
>>  } MXFWrappingIndicatorType;
>>
>>  typedef struct MXFLocalTagPair {
>> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
>> index 3bf480a3a6..7024d2ea7d 100644
>> --- a/libavformat/mxfdec.c
>> +++ b/libavformat/mxfdec.c
>> @@ -1413,6 +1413,7 @@ static void *mxf_resolve_strong_ref(MXFContext *mxf, 
UID *strong_ref, enum MXFMe
>>
>>  static const MXFCodecUL mxf_picture_essence_container_uls[] = {
>>      // video essence container uls
>> +    { { 
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c,0x06,0x00 }, 
15,   AV_CODEC_ID_JPEG2000, NULL, 16, AlwaysFrameWrap }, /*  MXF-GC P1 Frame-Wrapped 
JPEG 2000 */

Setting wrapping_indicator_pos to 16 seems problematic, it might cause
overreads in mxf_get_wrapping_kind, and a bit misleading...

I'd rather not add a new entry with the same codec id for the same
prefix, but add a JPEG2000Wrap MXFWrappingIndicatorType instead, and
similarly how D10D11Wrap is handled, add a new case and a simple if in
mxf_get_wrapping_kind:

case JPEG2000Wrap:
    if (val == 0x06)
        val = 0x01;
     break;

Thanks,
Marton


>>      { { 
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c,0x01,0x00 }, 
14,   AV_CODEC_ID_JPEG2000, NULL, 14 },
>>      { { 
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x10,0x60,0x01 }, 
14,       AV_CODEC_ID_H264, NULL, 15 }, /* H.264 */
>>      { { 
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x11,0x01,0x00 }, 
14,      AV_CODEC_ID_DNXHD, NULL, 14 }, /* VC-3 */
>> @@ -1497,6 +1498,9 @@ static MXFWrappingScheme mxf_get_wrapping_kind(UID 
*essence_container_ul)
>>              if (val == 0x02)
>>                  val = 0x01;
>>              break;
>> +        case AlwaysFrameWrap:
>> +            val = 0x01;
>> +            break;
>
> Looks OK. Still passes FATE.
>
> /Tomas
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to