On Sat, Jan 14, 2023 at 04:48:10PM +0100, Jerome Martinez wrote:
> Before the patch:
> - stored values were rounded to upper 16 multiple also for formats not using
> macroblocks (should be st->codecpar->width and st->codecpar->height when not
> MPEG formats; note that I found no other muxer doing the rounding for AVC,
> only for MPEG-2 Video, but I find no reason in specs for doing the
> difference so I kept the rounding for AVC)
> - sampled and displayed widths were stored width (should be
> st->codecpar->width like it is already done for height, with the DV50/100
> exception)
> 
> Could be tested with e.g.
> - fixed stored width (1912 instead of 1920) and height (1080 instead of
> 1088) not multiple of 16 :
> ffmpeg -f lavfi -i testsrc=duration=1:size=1912x1080 -c:v jpeg2000
> test_prores.mxf
> - fixed sampled/displayed width (1912 instead of 1920):
> ffmpeg -f lavfi -i testsrc=duration=1:size=1912x1080 -c:v mpeg2video
> test_mpeg2video.mxf

>  mxfenc.c |   28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> ff6497b7983ccb5ec7555b55ad0040a2c065d6d3  0001-avformat-mxfenc-fix-stored-sampled-displayed-width-h.patch
> From cda353059886182aab2e258023c4d027c448344b 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
> Sampled and displayed widths are codecpar ones (with DV exception)
> ---
>  libavformat/mxfenc.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index 58c551c83c..0b7e83ba4d 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,25 @@ 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_DVVIDEO:
> +    case AV_CODEC_ID_H264:
> +        //Based on 16x16 macroblocks
> +        stored_width = (stored_width+15)/16*16;

> +        stored_height = (stored_height+15)/16*16;

If this is supposed to match the actual macroblocks, then this would
have to consider field pictures and interlacing as it differs from
progressive

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin