[FFmpeg-devel] [PATCH 1/2] avformat/matroskadec: set aspect ratio only when DisplayWidth and DisplayHeight are in pixels

[James Almer]

On 10/16/2016 2:01 PM, Nicolas George wrote:
> Le quintidi 25 vendémiaire, an CCXXV, James Almer a écrit :
>>> 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?
> 
>>> 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".
> 
>>> -            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".
> 
> The logic seems fine, and a range comparison is probably better indeed.
> 
> By the way, in this spec:
> https://www.matroska.org/technical/specs/index.html
> I only see values 0-3, not 4 for unknown. Is there a more authoritative
> version of the spec?
> 
> Regards,

Yes, https://github.com/Matroska-Org/matroska-specification is afaik the
best version of the spec since it's the one that's going to be standardized.
The website should eventually get synced with it, though.

4 is a new value added to the element recently, which works in a backwards
compatible way as demuxers should consider anything above 3 as "unknown"
anyway, as per the spec guidelines.

https://mailarchive.ietf.org/arch/msg/cellar/x1F00MwqytPjrcNru6Kk2CWQ474
https://github.com/Matroska-Org/matroska-specification/pull/34