diff mbox series

[FFmpeg-devel] mxfdec.c: fixed frame wrapping detection for MXFGCP1FrameWrappedPicture essence container

Message ID 20210711164750.26005-1-pal@sandflow.com
State New
Headers show
Series [FFmpeg-devel] mxfdec.c: fixed frame wrapping detection for MXFGCP1FrameWrappedPicture essence container | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Pierre-Anthony Lemieux July 11, 2021, 4:47 p.m. UTC 
From: Pierre-Anthony Lemieux <pal@sandflow.com>

Signed-off-by: Pierre-Anthony Lemieux <pal@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.

 libavformat/mxf.h    | 3 ++-
 libavformat/mxfdec.c | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Tomas Härdin July 12, 2021, 11:01 a.m. UTC | #1 
sön 2021-07-11 klockan 09:47 -0700 skrev pal@sandflow.com:
> From: Pierre-Anthony Lemieux <pal@sandflow.com>
> 
> Signed-off-by: Pierre-Anthony Lemieux <pal@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.
> 
>  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 */
>      { { 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
Marton Balint July 17, 2021, 6:18 p.m. UTC | #2 
On Mon, 12 Jul 2021, Tomas Härdin wrote:

> sön 2021-07-11 klockan 09:47 -0700 skrev pal@sandflow.com:
>> From: Pierre-Anthony Lemieux <pal@sandflow.com>
>> 
>> Signed-off-by: Pierre-Anthony Lemieux <pal@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-request@ffmpeg.org with subject "unsubscribe".
Pierre-Anthony Lemieux July 17, 2021, 8:09 p.m. UTC | #3 
You mean something like this?

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 <cus@passwd.hu> wrote:
>
>
>
> On Mon, 12 Jul 2021, Tomas Härdin wrote:
>
> > sön 2021-07-11 klockan 09:47 -0700 skrev pal@sandflow.com:
> >> From: Pierre-Anthony Lemieux <pal@sandflow.com>
> >>
> >> Signed-off-by: Pierre-Anthony Lemieux <pal@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-request@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-request@ffmpeg.org with subject "unsubscribe".
Marton Balint July 18, 2021, 7:07 a.m. UTC | #4 
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 <cus@passwd.hu> wrote:
>>
>>
>>
>> On Mon, 12 Jul 2021, Tomas Härdin wrote:
>>
>> > sön 2021-07-11 klockan 09:47 -0700 skrev pal@sandflow.com:
>> >> From: Pierre-Anthony Lemieux <pal@sandflow.com>
>> >>
>> >> Signed-off-by: Pierre-Anthony Lemieux <pal@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-request@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-request@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-request@ffmpeg.org with subject "unsubscribe".
Pierre-Anthony Lemieux July 18, 2021, 6:09 p.m. UTC | #5 
Ok. Thanks. Submitted V2 of the patch at:

http://ffmpeg.org/pipermail/ffmpeg-devel/2021-July/282414.html

On Sun, Jul 18, 2021 at 12:07 AM Marton Balint <cus@passwd.hu> wrote:
>
>
>
> 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 <cus@passwd.hu> wrote:
> >>
> >>
> >>
> >> On Mon, 12 Jul 2021, Tomas Härdin wrote:
> >>
> >> > sön 2021-07-11 klockan 09:47 -0700 skrev pal@sandflow.com:
> >> >> From: Pierre-Anthony Lemieux <pal@sandflow.com>
> >> >>
> >> >> Signed-off-by: Pierre-Anthony Lemieux <pal@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-request@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-request@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-request@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-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

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 */
     { { 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;
     }
     if (val == 0x01)
         return FrameWrapped;