diff mbox

[FFmpeg-devel,06/13] avformat/mxfenc: Add Sample width/height/x offset/y offset, Display x offset and F2 offset

Message ID 20180507103817.8320-6-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer May 7, 2018, 10:38 a.m. UTC 
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/mxfenc.c            | 39 +++++++++++++++++++++++++++++++--
 tests/ref/fate/copy-trac4914    |  4 ++--
 tests/ref/fate/mxf-reel_name    |  2 +-
 tests/ref/fate/time_base        |  2 +-
 tests/ref/lavf/mxf              | 12 +++++-----
 tests/ref/lavf/mxf_d10          |  4 ++--
 tests/ref/lavf/mxf_dv25         |  4 ++--
 tests/ref/lavf/mxf_dvcpro50     |  4 ++--
 tests/ref/lavf/mxf_opatom       |  4 ++--
 tests/ref/lavf/mxf_opatom_audio |  4 ++--
 10 files changed, 57 insertions(+), 22 deletions(-)

Comments

Tomas Härdin May 8, 2018, 10:40 a.m. UTC | #1 
mån 2018-05-07 klockan 12:38 +0200 skrev Michael Niedermayer:
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> +    if (sc->interlaced) {
> +        //Display F2 Offset
> +        mxf_write_local_tag(pb, 4, 0x3217);
> +        avio_wb32(pb, -((st->codecpar->height - display_height)&1));

Negative values for DisplayF2Offset are not valid

/Tomas
Michael Niedermayer May 8, 2018, 4:26 p.m. UTC | #2 
On Tue, May 08, 2018 at 12:40:49PM +0200, Tomas Härdin wrote:
> mån 2018-05-07 klockan 12:38 +0200 skrev Michael Niedermayer:
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > +    if (sc->interlaced) {
> > +        //Display F2 Offset
> > +        mxf_write_local_tag(pb, 4, 0x3217);
> > +        avio_wb32(pb, -((st->codecpar->height - display_height)&1));
> 
> Negative values for DisplayF2Offset are not valid

The specification (SMPTE 377-1-2009) says:

The DisplayF2Offset Property adjusts the DisplayYOffset for the second field relative to that for the first field. Its
value shall be zero (0) or minus 1. A value of minus 1 shall invert the Displayed Topness relative to the Sampled
Topness.


[...]
Tobias Rapp May 9, 2018, 7:04 a.m. UTC | #3 
On 08.05.2018 12:40, Tomas Härdin wrote:
> mån 2018-05-07 klockan 12:38 +0200 skrev Michael Niedermayer:
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>> +    if (sc->interlaced) {
>> +        //Display F2 Offset
>> +        mxf_write_local_tag(pb, 4, 0x3217);
>> +        avio_wb32(pb, -((st->codecpar->height - display_height)&1));
> 
> Negative values for DisplayF2Offset are not valid

Can't say anything about the formula but according to The MXF Book (ISBN 
978-0240806938, page 187) the DisplayF2Offset property is defined as 
Int32, not UInt32.

Regards,
Tobias
Tomas Härdin May 10, 2018, 8:48 p.m. UTC | #4 
tis 2018-05-08 klockan 18:26 +0200 skrev Michael Niedermayer:
> On Tue, May 08, 2018 at 12:40:49PM +0200, Tomas Härdin wrote:
> > mån 2018-05-07 klockan 12:38 +0200 skrev Michael Niedermayer:
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > 
> > > +    if (sc->interlaced) {
> > > +        //Display F2 Offset
> > > +        mxf_write_local_tag(pb, 4, 0x3217);
> > > +        avio_wb32(pb, -((st->codecpar->height -
> > > display_height)&1));
> > 
> > Negative values for DisplayF2Offset are not valid
> 
> The specification (SMPTE 377-1-2009) says:
> 
> The DisplayF2Offset Property adjusts the DisplayYOffset for the
> second field relative to that for the first field. Its
> value shall be zero (0) or minus 1. A value of minus 1 shall invert
> the Displayed Topness relative to the Sampled
> Topness.

Huh, the document I have is from 2004, which says "If the property is
not present, its value shall be assumed to be 0. Valid values are zero
or 1.". So I guess they changed that. I guess it's fine then.

/Tomas
Michael Niedermayer May 10, 2018, 9:32 p.m. UTC | #5 
On Thu, May 10, 2018 at 10:48:59PM +0200, Tomas Härdin wrote:
> tis 2018-05-08 klockan 18:26 +0200 skrev Michael Niedermayer:
> > On Tue, May 08, 2018 at 12:40:49PM +0200, Tomas Härdin wrote:
> > > mån 2018-05-07 klockan 12:38 +0200 skrev Michael Niedermayer:
> > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > 
> > > > +    if (sc->interlaced) {
> > > > +        //Display F2 Offset
> > > > +        mxf_write_local_tag(pb, 4, 0x3217);
> > > > +        avio_wb32(pb, -((st->codecpar->height -
> > > > display_height)&1));
> > > 
> > > Negative values for DisplayF2Offset are not valid
> > 
> > The specification (SMPTE 377-1-2009) says:
> > 
> > The DisplayF2Offset Property adjusts the DisplayYOffset for the
> > second field relative to that for the first field. Its
> > value shall be zero (0) or minus 1. A value of minus 1 shall invert
> > the Displayed Topness relative to the Sampled
> > Topness.
> 
> Huh, the document I have is from 2004, which says "If the property is
> not present, its value shall be assumed to be 0. Valid values are zero
> or 1.". So I guess they changed that. I guess it's fine then.

will apply

thanks

[...]
diff mbox

Patch

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 9140302b81..e79e1ec7e6 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -480,9 +480,16 @@  static const MXFLocalTagPair mxf_local_tag_batch[] = {
     { 0x320D, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x04,0x01,0x03,0x02,0x05,0x00,0x00,0x00}}, /* Video Line Map */
     { 0x3203, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x02,0x02,0x00,0x00,0x00}}, /* Stored Width */
     { 0x3202, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x02,0x01,0x00,0x00,0x00}}, /* Stored Height */
+    { 0x3216, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x04,0x01,0x03,0x02,0x08,0x00,0x00,0x00}}, /* Stored F2 Offset */
+    { 0x3205, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x01,0x08,0x00,0x00,0x00}}, /* Sampled Width */
+    { 0x3204, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x01,0x07,0x00,0x00,0x00}}, /* Sampled Height */
+    { 0x3206, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x01,0x09,0x00,0x00,0x00}}, /* Sampled X Offset */
+    { 0x3207, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x01,0x0A,0x00,0x00,0x00}}, /* Sampled Y Offset */
     { 0x3209, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x01,0x0C,0x00,0x00,0x00}}, /* Display Width */
     { 0x3208, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x01,0x0B,0x00,0x00,0x00}}, /* Display Height */
+    { 0x320A, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x01,0x0D,0x00,0x00,0x00}}, /* Display X offset */
     { 0x320B, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x05,0x01,0x0E,0x00,0x00,0x00}}, /* Presentation Y offset */
+    { 0x3217, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x04,0x01,0x03,0x02,0x07,0x00,0x00,0x00}}, /* Display F2 offset */
     { 0x320E, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x00,0x00,0x00}}, /* Aspect Ratio */
     { 0x3201, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x04,0x01,0x06,0x01,0x00,0x00,0x00,0x00}}, /* Picture Essence Coding */
     { 0x3212, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x04,0x01,0x03,0x01,0x06,0x00,0x00,0x00}}, /* Field Dominance (Opt) */
@@ -1135,11 +1142,13 @@  static void mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID ke
     int stored_height = (st->codecpar->height+15)/16*16;
     int display_height;
     int f1, f2;
-    unsigned desc_size = size+8+8+8+8+8+8+8+5+16+4+12+20+5;
+    unsigned desc_size = size+8+8+8+8+8+8+8+5+16+4+12+20+5 + 5*8;
     if (sc->interlaced && sc->field_dominance)
         desc_size += 5;
     if (sc->signal_standard)
         desc_size += 5;
+    if (sc->interlaced)
+        desc_size += 8;
 
     mxf_write_generic_desc(s, st, key, desc_size);
 
@@ -1149,6 +1158,22 @@  static void mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID ke
     mxf_write_local_tag(pb, 4, 0x3202);
     avio_wb32(pb, stored_height>>sc->interlaced);
 
+    //Sampled width
+    mxf_write_local_tag(pb, 4, 0x3205);
+    avio_wb32(pb, st->codecpar->width);
+
+    //Samples height
+    mxf_write_local_tag(pb, 4, 0x3204);
+    avio_wb32(pb, st->codecpar->height>>sc->interlaced);
+
+    //Sampled X Offset
+    mxf_write_local_tag(pb, 4, 0x3206);
+    avio_wb32(pb, 0);
+
+    //Sampled Y Offset
+    mxf_write_local_tag(pb, 4, 0x3207);
+    avio_wb32(pb, 0);
+
     mxf_write_local_tag(pb, 4, 0x3209);
     avio_wb32(pb, st->codecpar->width);
 
@@ -1162,10 +1187,20 @@  static void mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID ke
     mxf_write_local_tag(pb, 4, 0x3208);
     avio_wb32(pb, display_height>>sc->interlaced);
 
-    // presentation Y offset
+    // display X offset
+    mxf_write_local_tag(pb, 4, 0x320A);
+    avio_wb32(pb, 0);
+
+    // display Y offset
     mxf_write_local_tag(pb, 4, 0x320B);
     avio_wb32(pb, (st->codecpar->height - display_height)>>sc->interlaced);
 
+    if (sc->interlaced) {
+        //Display F2 Offset
+        mxf_write_local_tag(pb, 4, 0x3217);
+        avio_wb32(pb, -((st->codecpar->height - display_height)&1));
+    }
+
     // component depth
     mxf_write_local_tag(pb, 4, 0x3301);
     avio_wb32(pb, sc->component_depth);
diff --git a/tests/ref/fate/copy-trac4914 b/tests/ref/fate/copy-trac4914
index 0d8f09176f..8f060eec38 100644
--- a/tests/ref/fate/copy-trac4914
+++ b/tests/ref/fate/copy-trac4914
@@ -1,5 +1,5 @@ 
-2bbcbc55eebf305aec776bce60d09f91 *tests/data/fate/copy-trac4914.mxf
-561209 tests/data/fate/copy-trac4914.mxf
+07932de23a28c175e259a8eb1cbfa052 *tests/data/fate/copy-trac4914.mxf
+561721 tests/data/fate/copy-trac4914.mxf
 #tb 0: 1001/30000
 #media_type 0: video
 #codec_id 0: rawvideo
diff --git a/tests/ref/fate/mxf-reel_name b/tests/ref/fate/mxf-reel_name
index b1a8840d7a..7d64903b72 100644
--- a/tests/ref/fate/mxf-reel_name
+++ b/tests/ref/fate/mxf-reel_name
@@ -1 +1 @@ 
-581d38fa877b2db15615989f335e9eaf
+acccd2f4898bcea3e0f68d8ca132e6ff
diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base
index 75ec4368a4..44db02ee59 100644
--- a/tests/ref/fate/time_base
+++ b/tests/ref/fate/time_base
@@ -1 +1 @@ 
-0979b614a34f668eb47278448b254000
+939abc1a1671fbbdab249ba944fe5b85
diff --git a/tests/ref/lavf/mxf b/tests/ref/lavf/mxf
index a09c8e1bab..600c976cc2 100644
--- a/tests/ref/lavf/mxf
+++ b/tests/ref/lavf/mxf
@@ -1,9 +1,9 @@ 
-e3f486ec383f9df4e4e7063959c88dd5 *./tests/data/lavf/lavf.mxf
-525881 ./tests/data/lavf/lavf.mxf
+64e49d7e44904049efb1085f0c8ebd51 *./tests/data/lavf/lavf.mxf
+526393 ./tests/data/lavf/lavf.mxf
 ./tests/data/lavf/lavf.mxf CRC=0x8dddfaab
-4de1237dea5f8377eed4c8effe037ffb *./tests/data/lavf/lavf.mxf
-561209 ./tests/data/lavf/lavf.mxf
+1bc3cd75f2e3a7e1b4586544fa015db1 *./tests/data/lavf/lavf.mxf
+561721 ./tests/data/lavf/lavf.mxf
 ./tests/data/lavf/lavf.mxf CRC=0xf21b1b48
-73dec65269c3f5ebe67e4e7fa6f2f6b7 *./tests/data/lavf/lavf.mxf
-525881 ./tests/data/lavf/lavf.mxf
+0d805c05a9b0078727b3e8797dcaea95 *./tests/data/lavf/lavf.mxf
+526393 ./tests/data/lavf/lavf.mxf
 ./tests/data/lavf/lavf.mxf CRC=0x8dddfaab
diff --git a/tests/ref/lavf/mxf_d10 b/tests/ref/lavf/mxf_d10
index 62b96b4ba6..ae29dcbc58 100644
--- a/tests/ref/lavf/mxf_d10
+++ b/tests/ref/lavf/mxf_d10
@@ -1,3 +1,3 @@ 
-ea6d7025d72df9aaf63bdbc2be8c82dc *./tests/data/lavf/lavf.mxf_d10
-5331501 ./tests/data/lavf/lavf.mxf_d10
+5074cc111fe19d9442e2a962dcf9e72d *./tests/data/lavf/lavf.mxf_d10
+5332013 ./tests/data/lavf/lavf.mxf_d10
 ./tests/data/lavf/lavf.mxf_d10 CRC=0x6c74d488
diff --git a/tests/ref/lavf/mxf_dv25 b/tests/ref/lavf/mxf_dv25
index 323abc330b..97b6807c93 100644
--- a/tests/ref/lavf/mxf_dv25
+++ b/tests/ref/lavf/mxf_dv25
@@ -1,3 +1,3 @@ 
-2f8cb1656178419950a9a3505cae3f5b *./tests/data/lavf/lavf.mxf_dv25
-3833901 ./tests/data/lavf/lavf.mxf_dv25
+9376aa790712ef9cfba1ec9e9bacbad9 *./tests/data/lavf/lavf.mxf_dv25
+3834413 ./tests/data/lavf/lavf.mxf_dv25
 ./tests/data/lavf/lavf.mxf_dv25 CRC=0xbdaf7f52
diff --git a/tests/ref/lavf/mxf_dvcpro50 b/tests/ref/lavf/mxf_dvcpro50
index 5e93737904..17870c87e3 100644
--- a/tests/ref/lavf/mxf_dvcpro50
+++ b/tests/ref/lavf/mxf_dvcpro50
@@ -1,3 +1,3 @@ 
-9377a3afbf431442e17c5d891b4e6252 *./tests/data/lavf/lavf.mxf_dvcpro50
-7430701 ./tests/data/lavf/lavf.mxf_dvcpro50
+5170950bbd38d1b9868dbcea1fbf1e01 *./tests/data/lavf/lavf.mxf_dvcpro50
+7431213 ./tests/data/lavf/lavf.mxf_dvcpro50
 ./tests/data/lavf/lavf.mxf_dvcpro50 CRC=0xe3bbe4b4
diff --git a/tests/ref/lavf/mxf_opatom b/tests/ref/lavf/mxf_opatom
index c5aac167b5..b04ef93111 100644
--- a/tests/ref/lavf/mxf_opatom
+++ b/tests/ref/lavf/mxf_opatom
@@ -1,3 +1,3 @@ 
-ebf818890f92bb9710798a3e2ab571fd *./tests/data/lavf/lavf.mxf_opatom
-4717113 ./tests/data/lavf/lavf.mxf_opatom
+8965bbb1801e9c299510592cfe575e44 *./tests/data/lavf/lavf.mxf_opatom
+4717625 ./tests/data/lavf/lavf.mxf_opatom
 ./tests/data/lavf/lavf.mxf_opatom CRC=0xf55aa22a
diff --git a/tests/ref/lavf/mxf_opatom_audio b/tests/ref/lavf/mxf_opatom_audio
index 08cb7168dd..69b52963df 100644
--- a/tests/ref/lavf/mxf_opatom_audio
+++ b/tests/ref/lavf/mxf_opatom_audio
@@ -1,3 +1,3 @@ 
-b79b636502d47239def6e85ab8cc06b3 *./tests/data/lavf/lavf.mxf_opatom_audio
-102457 ./tests/data/lavf/lavf.mxf_opatom_audio
+72c84169d102d91095bdfaf374e1b0f3 *./tests/data/lavf/lavf.mxf_opatom_audio
+102969 ./tests/data/lavf/lavf.mxf_opatom_audio
 ./tests/data/lavf/lavf.mxf_opatom_audio CRC=0xd155c6ff