[FFmpeg-devel] [PATCH] avformat/mxfenc: Add color siting element
tim nicholson
nichot20 at yahoo.com
Tue May 19 13:07:24 CEST 2015
On 19/05/15 01:33, Michael Niedermayer wrote:
> The default is assumed to be 0xFF, which is what the 2009 spec lists,
> the older version i have lists 0 as the default.
>
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
> libavformat/mxfenc.c | 28 +++++++++++++++++++++++++
> tests/ref/lavf/mxf | 12 +++++------
> tests/ref/lavf/mxf_d10 | 2 +-
> tests/ref/lavf/mxf_opatom | 2 +-
> tests/ref/lavf/mxf_opatom_audio | 2 +-
> tests/ref/seek/lavf-mxf | 44 +++++++++++++++++++--------------------
> 6 files changed, 59 insertions(+), 31 deletions(-)
>
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index 659c34f..0af3db1 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -79,6 +79,7 @@ typedef struct MXFStreamContext {
> int interlaced; ///< whether picture is interlaced
> int field_dominance; ///< tff=1, bff=2
> int component_depth;
> + int color_siting;
> int h_chroma_sub_sample;
> int temporal_reordering;
> AVRational aspect_ratio; ///< display aspect ratio
> @@ -416,6 +417,7 @@ static const MXFLocalTagPair mxf_local_tag_batch[] = {
> // CDCI Picture Essence Descriptor
> { 0x3301, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x04,0x01,0x05,0x03,0x0A,0x00,0x00,0x00}}, /* Component Depth */
> { 0x3302, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x01,0x05,0x00,0x00,0x00}}, /* Horizontal Subsampling */
> + { 0x3303, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x01,0x06,0x00,0x00,0x00}}, /* Color Siting */
> // Generic Sound Essence Descriptor
> { 0x3D02, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x04,0x04,0x02,0x03,0x01,0x04,0x00,0x00,0x00}}, /* Locked/Unlocked */
> { 0x3D03, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x04,0x02,0x03,0x01,0x01,0x01,0x00,0x00}}, /* Audio sampling rate */
> @@ -996,6 +998,8 @@ static void mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID ke
> unsigned desc_size = size+8+8+8+8+8+8+8+5+16+sc->interlaced*4+12+20;
> if (sc->interlaced && sc->field_dominance)
> desc_size += 5;
> + if (sc->color_siting != 0xFF)
> + desc_size += 5;
>
> mxf_write_generic_desc(s, st, key, desc_size);
>
> @@ -1030,6 +1034,12 @@ static void mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID ke
> mxf_write_local_tag(pb, 4, 0x3302);
> avio_wb32(pb, sc->h_chroma_sub_sample);
>
> + if (sc->color_siting != 0xFF) {
> + // color siting
> + mxf_write_local_tag(pb, 1, 0x3303);
> + avio_w8(pb, sc->color_siting);
> + }
> +
If the value as far as we can determine is "unknown", should we not
write that value (0xFF), rather than leave the metadata out, and hope
that any reader will assume the 2009 default rather than the previous,
different default? It would seem to me to be more universal.
I'm generally wary of leaving out values just because they are some
default especially if that default is subject to change.
> // frame layout
> mxf_write_local_tag(pb, 1, 0x320C);
> avio_w8(pb, sc->interlaced);
> @@ -2037,11 +2047,29 @@ static int mxf_write_header(AVFormatContext *s)
> // Default component depth to 8
> sc->component_depth = 8;
> sc->h_chroma_sub_sample = 2;
> + sc->color_siting = 0xFF;
>
> if (pix_desc) {
> sc->component_depth = pix_desc->comp[0].depth_minus1 + 1;
> sc->h_chroma_sub_sample = 1 << pix_desc->log2_chroma_w;
> }
> + switch (st->codec->chroma_sample_location) {
> + case AVCHROMA_LOC_TOPLEFT: sc->color_siting = 0; break;
> + case AVCHROMA_LOC_LEFT: sc->color_siting = 6; break;
> + case AVCHROMA_LOC_TOP: sc->color_siting = 1; break;
> + case AVCHROMA_LOC_CENTER: sc->color_siting = 3; break;
If these mappings are correct, then the AVCHROMA_LOC_ names are
certainly not intuitive (and the comments unhelpful), and bear little
relation to the way SMPTE S377 describes the positioning!
e.g.
pixfmt.h line 541->
AVCHROMA_LOC_TOPLEFT = 3, ///< DV
SMPTE S377 page 165->
00h coSiting...as in ITU-R Rec 601, SMPTE 314M 4:1:1 or MPEG-2 4:2:2.
> + case AVCHROMA_LOC_UNSPECIFIED:
> + if (pix_desc) {
> + if (pix_desc->log2_chroma_h == 0) {
> + sc->color_siting = 0;
> + } else if (pix_desc->log2_chroma_w == 1 && pix_desc->log2_chroma_h == 1) {
> + switch (st->codec->codec_id) {
> + case AV_CODEC_ID_MPEG2VIDEO: sc->color_siting = 6; break;
Only true for 4:2:0 mpeg2, what about 4:2:2 (XDCAM-HD) or does the 'if'
filter that out? (I'm not that familiar with the pix_desc struct).
> + }
> + }
> + }
> + break;
> + }
>
> mxf->timecode_base = (tbc.den + tbc.num/2) / tbc.num;
> spf = ff_mxf_get_samples_per_frame(s, tbc);
> diff --git a/tests/ref/lavf/mxf b/tests/ref/lavf/mxf
> index 5fb984b..4885bee 100644
> --- a/tests/ref/lavf/mxf
> +++ b/tests/ref/lavf/mxf
> @@ -1,9 +1,9 @@
> -613b160bf09927661d9455dd975ad780 *./tests/data/lavf/lavf.mxf
> -525369 ./tests/data/lavf/lavf.mxf
> +bda6be285b4f275b6d8a74e1c5c5aec9 *./tests/data/lavf/lavf.mxf
> +525881 ./tests/data/lavf/lavf.mxf
> ./tests/data/lavf/lavf.mxf CRC=0xdbfff6f1
> -64471cfc480751f2ca7998c4edbc7a02 *./tests/data/lavf/lavf.mxf
> -560697 ./tests/data/lavf/lavf.mxf
> +a93989cd8e80e9edcce60a2f01eb068b *./tests/data/lavf/lavf.mxf
> +561209 ./tests/data/lavf/lavf.mxf
> ./tests/data/lavf/lavf.mxf CRC=0x11a6178e
> -a546bd6d8fe1608690bf2fc326cab0c3 *./tests/data/lavf/lavf.mxf
> -525369 ./tests/data/lavf/lavf.mxf
> +c0b9255defc9f0494084e2b286f222b9 *./tests/data/lavf/lavf.mxf
> +525881 ./tests/data/lavf/lavf.mxf
> ./tests/data/lavf/lavf.mxf CRC=0xdbfff6f1
> diff --git a/tests/ref/lavf/mxf_d10 b/tests/ref/lavf/mxf_d10
> index 8201557..9324465 100644
> --- a/tests/ref/lavf/mxf_d10
> +++ b/tests/ref/lavf/mxf_d10
> @@ -1,3 +1,3 @@
> -22dca5c8d62fe7ad1c56416ae736d6c1 *./tests/data/lavf/lavf.mxf_d10
> +bb8f8e2661e1c81fe4e1ce7900b54200 *./tests/data/lavf/lavf.mxf_d10
> 5330989 ./tests/data/lavf/lavf.mxf_d10
> ./tests/data/lavf/lavf.mxf_d10 CRC=0x6c74d488
> diff --git a/tests/ref/lavf/mxf_opatom b/tests/ref/lavf/mxf_opatom
> index 0dd59c1..9c9ab15 100644
> --- a/tests/ref/lavf/mxf_opatom
> +++ b/tests/ref/lavf/mxf_opatom
> @@ -1,3 +1,3 @@
> -5df0bb1083cbe0ef3a70e4a93e44d812 *./tests/data/lavf/lavf.mxf_opatom
> +2c1a3b94af9e7c606532b1b636902b3a *./tests/data/lavf/lavf.mxf_opatom
> 4717113 ./tests/data/lavf/lavf.mxf_opatom
> ./tests/data/lavf/lavf.mxf_opatom CRC=0xbdd696b9
> diff --git a/tests/ref/lavf/mxf_opatom_audio b/tests/ref/lavf/mxf_opatom_audio
> index 45010d0..2f79246 100644
> --- a/tests/ref/lavf/mxf_opatom_audio
> +++ b/tests/ref/lavf/mxf_opatom_audio
> @@ -1,3 +1,3 @@
> -77bd25ba3213ffbdc4d4c9052914510f *./tests/data/lavf/lavf.mxf_opatom_audio
> +d571768dd4e32b56917fb76abdb40570 *./tests/data/lavf/lavf.mxf_opatom_audio
> 102457 ./tests/data/lavf/lavf.mxf_opatom_audio
> ./tests/data/lavf/lavf.mxf_opatom_audio CRC=0xd155c6ff
> diff --git a/tests/ref/seek/lavf-mxf b/tests/ref/seek/lavf-mxf
> index 9b23466..f1aaa19 100644
> --- a/tests/ref/seek/lavf-mxf
> +++ b/tests/ref/seek/lavf-mxf
> @@ -1,48 +1,48 @@
> -ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6144 size: 24801
> +ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6656 size: 24801
> ret: 0 st:-1 flags:0 ts:-1.000000
> -ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6144 size: 24801
> +ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6656 size: 24801
> ret: 0 st:-1 flags:1 ts: 1.894167
> -ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460288 size: 24711
> +ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460800 size: 24711
> ret: 0 st: 0 flags:0 ts: 0.800000
> -ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460288 size: 24711
> +ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460800 size: 24711
> ret: 0 st: 0 flags:1 ts:-0.320000
> -ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6144 size: 24801
> +ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6656 size: 24801
> ret:-1 st: 1 flags:0 ts: 2.576667
> ret: 0 st: 1 flags:1 ts: 1.470833
> -ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460288 size: 24711
> +ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460800 size: 24711
> ret: 0 st:-1 flags:0 ts: 0.365002
> -ret: 0 st: 0 flags:1 dts: 0.360000 pts: 0.480000 pos: 211456 size: 24786
> +ret: 0 st: 0 flags:1 dts: 0.360000 pts: 0.480000 pos: 211968 size: 24786
> ret: 0 st:-1 flags:1 ts:-0.740831
> -ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6144 size: 24801
> +ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6656 size: 24801
> ret:-1 st: 0 flags:0 ts: 2.160000
> ret: 0 st: 0 flags:1 ts: 1.040000
> -ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460288 size: 24711
> +ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460800 size: 24711
> ret: 0 st: 1 flags:0 ts:-0.058333
> -ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6144 size: 24801
> +ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6656 size: 24801
> ret: 0 st: 1 flags:1 ts: 2.835833
> -ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460288 size: 24711
> +ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460800 size: 24711
> ret:-1 st:-1 flags:0 ts: 1.730004
> ret: 0 st:-1 flags:1 ts: 0.624171
> -ret: 0 st: 0 flags:1 dts: 0.360000 pts: 0.480000 pos: 211456 size: 24786
> +ret: 0 st: 0 flags:1 dts: 0.360000 pts: 0.480000 pos: 211968 size: 24786
> ret: 0 st: 0 flags:0 ts:-0.480000
> -ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6144 size: 24801
> +ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6656 size: 24801
> ret: 0 st: 0 flags:1 ts: 2.400000
> -ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460288 size: 24711
> +ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460800 size: 24711
> ret:-1 st: 1 flags:0 ts: 1.306667
> ret: 0 st: 1 flags:1 ts: 0.200833
> -ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6144 size: 24801
> +ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6656 size: 24801
> ret: 0 st:-1 flags:0 ts:-0.904994
> -ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6144 size: 24801
> +ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6656 size: 24801
> ret: 0 st:-1 flags:1 ts: 1.989173
> -ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460288 size: 24711
> +ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460800 size: 24711
> ret: 0 st: 0 flags:0 ts: 0.880000
> -ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460288 size: 24711
> +ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460800 size: 24711
> ret: 0 st: 0 flags:1 ts:-0.240000
> -ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6144 size: 24801
> +ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6656 size: 24801
> ret:-1 st: 1 flags:0 ts: 2.671667
> ret: 0 st: 1 flags:1 ts: 1.565833
> -ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460288 size: 24711
> +ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460800 size: 24711
> ret: 0 st:-1 flags:0 ts: 0.460008
> -ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460288 size: 24711
> +ret: 0 st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460800 size: 24711
> ret: 0 st:-1 flags:1 ts:-0.645825
> -ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6144 size: 24801
> +ret: 0 st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos: 6656 size: 24801
>
--
Tim.
Key Fingerprint 38CF DB09 3ED0 F607 8B67 6CED 0C0B FC44 8B0B FC83
More information about the ffmpeg-devel
mailing list