bbc / bmx

Library and utilities to read and write broadcasting media files. Primarily supports the MXF file format
BSD 3-Clause "New" or "Revised" License
57 stars 16 forks source link

mxf2raw: extend `--rdd6` option description #45

Closed philipnbbc closed 7 months ago

philipnbbc commented 9 months ago

Fixes https://github.com/bbc/bmx/issues/44

thomasheritage commented 9 months ago

Thanks, it's very useful to have this documented more thoroughly.

A few thoughts:

philipnbbc commented 9 months ago

I've updated the PR to

The RDD 6 XML schema defines the desc_text element to have at most 8 programs. See table 6.2 of the RDD 6 spec.. E.g. if there is 5.1 and stereo audio then that is represented as 2 RDD 6 programs.

thomasheritage commented 7 months ago

That all sounds very good, thanks for all the improvements.

Just one outstanding item:

presumably mxf2raw only reads RDD 6 metadata (except for those two fields) from the first frame indicated in the frames commandline parameter? i.e. won't notice if other values actually vary

On this topic, the help text says:

The output XML will contain the static RDD 6 metadata

Perhaps you can just add something to convey whether this RDD 6 metadata being static is just an assumption, or whether mxf2raw actually checks that it's static (for the frames that it reads)?

philipnbbc commented 7 months ago

Just one outstanding item:

presumably mxf2raw only reads RDD 6 metadata (except for those two fields) from the first frame indicated in the frames commandline parameter? i.e. won't notice if other values actually vary

Yes, it assumes it is static and doesn't check if it isn't.

On this topic, the help text says:

The output XML will contain the static RDD 6 metadata

Perhaps you can just add something to convey whether this RDD 6 metadata being static is just an assumption, or whether mxf2raw actually checks that it's static (for the frames that it reads)?

I've changed it to say it assumes it is static and that it uses the first and subsequent frames to accumulate the description text.

thomasheritage commented 7 months ago

LGTM