On 10/16/2016 10:28 AM, James Almer wrote:
> 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?
> 
> 
> 0001-Partially-revert-avformat-matroskadec-set-aspect-rat.patch
> 
> 
> 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) {

Alternatively, i could make it check for "less than" rather than "not equal",
since values that are not defined (5 and above) should be ignored as they are,
as per the spec guidelines, "unknown".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to