[FFmpeg-devel] avformat/mxfenc: fix stored/sampled/displayed width/height
Jerome Martinez
jerome at mediaarea.net
Tue Mar 14 11:41:41 EET 2023
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
-------------- next part --------------
From c93c73164d427b2bcd29744b5a26ab82b1fe223a Mon Sep 17 00:00:00 2001
From: Jerome Martinez <jerome at 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
More information about the ffmpeg-devel
mailing list