Re: [FFmpeg-devel] avformat/mxfenc: fix stored/sampled/displayed width/height

[Jerome Martinez <jerome@mediaarea.net>]

[-- Attachment #1: Type: text/plain, Size: 1352 bytes --]

On 10/03/2023 22:10, Marton Balint wrote:
>
>
> On Mon, 6 Mar 2023, Nicolas Gaullier wrote:
>
> [...]
>>
>> Some weeks later now and no replies, maybe time to go on ?
>> I think the "case AV_CODEC_ID_DVVIDEO:" can be removed as discussed, 
>> fate updated and that should be ok for everybody.
>> (Ideally, it could have been an opportunity to document why we have 
>> this "DV exception", but I understand it is not very comfortable to 
>> write as there is no meaningful reason, so forget about this, this 
>> won't hold up the patch anyway)
>> For information, there was a long thread recently on ffmpeg-user 
>> about a "bug" in dnxhd stored_height (will be fixed with your patch):
>> https://ffmpeg.org/pipermail/ffmpeg-user/2023-February/056111.html
>
> Will apply the patch in a couple of days unless somebody objects. If 
> you want to change DV height (seems reasonable), please send a follow 
> up patch with fate updates after that.


Apologizes for the huge delay.
Attached is an updated patch.

I changed the DV part (also removed from the 16x16 macroblock thing)
I added some comments about specs (summary: DNxHD is explicit, others 
are not but implementation I know don't do the 16x16 macroblock thing).
The FATE tests for DV are not impacted because they are SD and SD 
width/height are multiple of 16 so I added a DV100 test.

Jérôme

[-- Attachment #2: 0001-avformat-mxfenc-fix-stored-sampled-displayed-width-h.patch --]
[-- Type: text/plain, Size: 6332 bytes --]

From c93c73164d427b2bcd29744b5a26ab82b1fe223a Mon Sep 17 00:00:00 2001
From: Jerome Martinez <jerome@mediaarea.net>
Date: Sat, 14 Jan 2023 13:32:36 +0100
Subject: [PATCH] avformat/mxfenc: fix stored/sampled/displayed width/height

Stored values are rounded to upper 16 multiple only for MPEG related formats (DV is not 16x16 macroblocks based, RDD 44 only refers to ST 377-1 without precision and no known implementation puts 1088 or 544, ST 2019-4 explicitly indicates 1080 for 1080p and 540 for 1080i)
Sampled and displayed widths are codecpar ones (with DV exception)
---
 libavformat/mxfenc.c          | 27 +++++++++++++++++++++------
 tests/fate/lavf-container.mak |  3 ++-
 tests/ref/lavf/mxf_dvcpro100  |  3 +++
 tests/ref/lavf/mxf_opatom     |  2 +-
 4 files changed, 27 insertions(+), 8 deletions(-)
 create mode 100644 tests/ref/lavf/mxf_dvcpro100

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 58c551c83c..614fc5ec89 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -1109,8 +1109,9 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
 {
     MXFStreamContext *sc = st->priv_data;
     AVIOContext *pb = s->pb;
-    int stored_width = 0;
-    int stored_height = (st->codecpar->height+15)/16*16;
+    int stored_width = st->codecpar->width;
+    int stored_height = st->codecpar->height;
+    int display_width;
     int display_height;
     int f1, f2;
     const MXFCodecUL *color_primaries_ul;
@@ -1129,12 +1130,24 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
         else if (st->codecpar->height == 720)
             stored_width = 1280;
     }
-    if (!stored_width)
-        stored_width = (st->codecpar->width+15)/16*16;
+    display_width = stored_width;
 
+    switch (st->codecpar->codec_id) {
+    case AV_CODEC_ID_MPEG2VIDEO:
+    case AV_CODEC_ID_H264:
+        //Based on 16x16 macroblocks
+        stored_width = (stored_width+15)/16*16;
+        stored_height = (stored_height+15)/16*16;
+        break;
+    default:
+        break;
+    }
+
+    //Stored width
     mxf_write_local_tag(s, 4, 0x3203);
     avio_wb32(pb, stored_width);
 
+    //Stored height
     mxf_write_local_tag(s, 4, 0x3202);
     avio_wb32(pb, stored_height>>sc->interlaced);
 
@@ -1154,7 +1167,7 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
 
     //Sampled width
     mxf_write_local_tag(s, 4, 0x3205);
-    avio_wb32(pb, stored_width);
+    avio_wb32(pb, display_width);
 
     //Samples height
     mxf_write_local_tag(s, 4, 0x3204);
@@ -1168,8 +1181,9 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
     mxf_write_local_tag(s, 4, 0x3207);
     avio_wb32(pb, 0);
 
+    //Display width
     mxf_write_local_tag(s, 4, 0x3209);
-    avio_wb32(pb, stored_width);
+    avio_wb32(pb, display_width);
 
     if (st->codecpar->height == 608) // PAL + VBI
         display_height = 576;
@@ -1178,6 +1192,7 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
     else
         display_height = st->codecpar->height;
 
+    //Display height
     mxf_write_local_tag(s, 4, 0x3208);
     avio_wb32(pb, display_height>>sc->interlaced);
 
diff --git a/tests/fate/lavf-container.mak b/tests/fate/lavf-container.mak
index 4bf7636b56..a04d31156a 100644
--- a/tests/fate/lavf-container.mak
+++ b/tests/fate/lavf-container.mak
@@ -8,7 +8,7 @@ FATE_LAVF_CONTAINER-$(call ENCDEC2, MPEG4,      MP2,       MATROSKA)           +
 FATE_LAVF_CONTAINER-$(call ENCDEC2, MPEG4,      PCM_ALAW,  MOV)                += mov mov_rtphint ismv
 FATE_LAVF_CONTAINER-$(call ENCDEC,  MPEG4,                 MOV)                += mp4
 FATE_LAVF_CONTAINER-$(call ENCDEC2, MPEG1VIDEO, MP2,       MPEG1SYSTEM MPEGPS) += mpg
-FATE_LAVF_CONTAINER-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF)                += mxf mxf_dv25 mxf_dvcpro50
+FATE_LAVF_CONTAINER-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF)                += mxf mxf_dv25 mxf_dvcpro50 mxf_dvcpro100
 FATE_LAVF_CONTAINER-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF_D10 MXF)        += mxf_d10
 FATE_LAVF_CONTAINER-$(call ENCDEC2, DNXHD,      PCM_S16LE, MXF_OPATOM MXF)     += mxf_opatom mxf_opatom_audio
 FATE_LAVF_CONTAINER-$(call ENCDEC2, MPEG4,      MP2,       NUT)                += nut
@@ -55,6 +55,7 @@ fate-lavf-mxf: CMD = lavf_container_timecode "-ar 48000 -bf 2 -threads 1"
 fate-lavf-mxf_d10: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,pad=720:608:0:32 -c:v mpeg2video -g 0 -flags +ildct+low_delay -dc 10 -non_linear_quant 1 -intra_vlc 1 -qscale 1 -ps 1 -qmin 1 -rc_max_vbv_use 1 -rc_min_vbv_use 1 -pix_fmt yuv422p -minrate 30000k -maxrate 30000k -b 30000k -bufsize 1200000 -top 1 -rc_init_occupancy 1200000 -qmax 12 -f mxf_d10"
 fate-lavf-mxf_dv25: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,setdar=4/3 -c:v dvvideo -pix_fmt yuv420p -b 25000k -top 0 -f mxf"
 fate-lavf-mxf_dvcpro50: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,setdar=16/9 -c:v dvvideo -pix_fmt yuv422p -b 50000k -top 0 -f mxf"
+fate-lavf-mxf_dvcpro100: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=1440:1080,setdar=16/9 -c:v dvvideo -pix_fmt yuv422p -b 100000k -top 0 -f mxf"
 fate-lavf-mxf_opatom: CMD = lavf_container "" "-s 1920x1080 -c:v dnxhd -pix_fmt yuv422p -vb 36M -f mxf_opatom -map 0"
 fate-lavf-mxf_opatom_audio: CMD = lavf_container "-ar 48000 -ac 1" "-f mxf_opatom -mxf_audio_edit_rate 25 -map 1"
 fate-lavf-smjpeg:  CMD = lavf_container "" "-f smjpeg"
diff --git a/tests/ref/lavf/mxf_dvcpro100 b/tests/ref/lavf/mxf_dvcpro100
new file mode 100644
index 0000000000..8193828e2c
--- /dev/null
+++ b/tests/ref/lavf/mxf_dvcpro100
@@ -0,0 +1,3 @@
+efa5cdde54ac38b02827ee8b6d7f03a4 *tests/data/lavf/lavf.mxf_dvcpro100
+14637613 tests/data/lavf/lavf.mxf_dvcpro100
+tests/data/lavf/lavf.mxf_dvcpro100 CRC=0x245e9a5f
diff --git a/tests/ref/lavf/mxf_opatom b/tests/ref/lavf/mxf_opatom
index 75f85b604e..fa72241813 100644
--- a/tests/ref/lavf/mxf_opatom
+++ b/tests/ref/lavf/mxf_opatom
@@ -1,3 +1,3 @@
-215ea72602bfeb70e99fc9d79fef073c *tests/data/lavf/lavf.mxf_opatom
+6418766ca9b8b23fae74a80d66f26f3e *tests/data/lavf/lavf.mxf_opatom
 4717625 tests/data/lavf/lavf.mxf_opatom
 tests/data/lavf/lavf.mxf_opatom CRC=0xb13ba2f8
-- 
2.13.3.windows.1


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".