[FFmpeg-devel] [PATCH] libavformat/mxfdec.c: Recognize and Ignore MXF fill boxes

[Tomas Härdin]

ons 2024-09-11 klockan 10:15 +0200 skrev Martin Schitter:
> This adds support for empty 'fill' boxes while decoding MXF files.
> ---
>  libavformat/mxfdec.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index 142b3e6..701fc9f 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -367,6 +367,8 @@ static const uint8_t
> mxf_mca_rfc5646_spoken_language[]     = { 0x06,0x0e,0x2b,0x
>  
>  static const uint8_t mxf_sub_descriptor[]                  = {
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x09,0x06,0x01,0x01,0x04,0x06,0x10
> ,0x00,0x00 };
>  
> +static const uint8_t mxf_fill_key[]                        = {
> 0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x03,0x01,0x02,0x10,0x01,0x00
> ,0x00,0x00 };
> +
>  static const uint8_t mxf_mastering_display_prefix[13]      = {
> FF_MXF_MasteringDisplay_PREFIX };
>  static const uint8_t mxf_mastering_display_uls[4][16] = {
>      FF_MXF_MasteringDisplayPrimaries,
> @@ -3730,6 +3732,11 @@ static int mxf_read_header(AVFormatContext *s)
>                  continue;
>          }
>  
> +        if (IS_KLV_KEY(klv.key, mxf_fill_key)){
> +            avio_skip(s->pb, klv.length);
> +            continue;
> +        }

This could also be done using mxf_metadata_read_table[] using a simple
stub callback.

Alternatively we could add some logic to the parsing loop that skips
KLVs whose key's entry in the table have read == NULL. The loop
termination condition would need to either change to checking if the
first byte of the UL is zero, or maybe just use FF_ARRAY_ELEMS. This
way we could add more keys to skip in the future. This is also possible
with the stub approach

Another thing we could do while we're at it is to add a matching length
to MXFMetadataReadTableEntry, which would allow simplifying the table

Also IS_KLV_KEY() seems wrong. It should skip the version byte I think
(I'd have to double-check the spec). That doesn't need to hold up this
patch of course

/Tomas