On 10/16/2016 5:58 AM, Nicolas George wrote: > Le quartidi 24 vendémiaire, an CCXXV, James Almer a écrit : >> A missing DisplayUnit element or one with the default value of 0 means >> DisplayWidth and DisplayHeight should be interpreted as pixels. >> >> The current code setting st->sample_aspect_ratio is wrong when DisplayUnit >> is anything else. > > Sorry to react after it was pushed, but: are you sure about the logic? > Naively, I think that a/b makes sense whatever the unit for a and b, as long > as it is known and the same: the logic should be applied for all units > except UNKNOWN. What am I missing?
Nothing probably, just me not giving it much thought after i couldn't find any sample using cm, in or dar instead of pixels, and assuming the calculations were tailored specifically for sizes in pixels. I manually created some and you're right it seems to work fine with all of them. Is the attached patch ok?
>From 14a71576f69a7fdb4fd48502ec2ea36c26297004 Mon Sep 17 00:00:00 2001 From: James Almer <jamr...@gmail.com> Date: Sun, 16 Oct 2016 10:13:45 -0300 Subject: [PATCH] Partially revert "avformat/matroskadec: set aspect ratio only when DisplayWidth and DisplayHeight are in pixels" The code works just fine regardless of unit, so only make sure DisplayUnit is not "unknown". Found-by: Nicolas George <geo...@nsup.org> Signed-off-by: James Almer <jamr...@gmail.com> --- libavformat/matroskadec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 0d17a7e..f9f54ff 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -2297,7 +2297,7 @@ static int matroska_parse_tracks(AVFormatContext *s) if (track->video.stereo_mode && track->video.stereo_mode < MATROSKA_VIDEO_STEREOMODE_TYPE_NB) mkv_stereo_mode_display_mul(track->video.stereo_mode, &display_width_mul, &display_height_mul); - if (track->video.display_unit == MATROSKA_VIDEO_DISPLAYUNIT_PIXELS) { + if (track->video.display_unit != MATROSKA_VIDEO_DISPLAYUNIT_UNKNOWN) { av_reduce(&st->sample_aspect_ratio.num, &st->sample_aspect_ratio.den, st->codecpar->height * track->video.display_width * display_width_mul, -- 2.9.1
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel