File__analyze_streams.cpp - Change for 2.39:1 DisplayAspectRatio/Stri… #829
Conversation
| @@ -2507,7 +2507,8 @@ void File__Analyze::DisplayAspectRatio_Fill(const Ztring &Value, stream_t Stream | |||
| else if (DAR>=(float)2.15 && DAR<(float)2.22) DARS=__T("2.2:1"); | |||
There was a problem hiding this comment.
Sorry, looking at your commit i just saw that 2.2:1 line above and get suspicious. i couldn't resist to post a remark here.
It look to me that it should be 2.21:1
+The following fixed values are supported:
+@table @option
+@item 4/3
+@item 16/9
+@item 221/100
+@EnD table
+Any other value will result in square pixels being signalled instead
+(see H.262 section 6.3.3 and table 6-3).
Taken from FFmpeg.
EDIT:
https://en.wikipedia.org/wiki/Aspect_ratio_(image)
2.20:1 (11:5, 22:10): 70 mm standard. Originally developed for Todd-AO in the 1950s. Specified in MPEG-2 as 2.21:1, but hardly used.
For the record, some popular movies like "2001", "Lawrence of Arabia" or "Tomorrowland" have been releasized in such format.
There was a problem hiding this comment.
Is this potentially outside of the scope of the pull request/issue which was dealing with the 2.39:1 issue?
There was a problem hiding this comment.
Hehe. As i said sorry, i couldn't resist when i discovered it ;)
You are right, i should have open an other issue. I'll try to remember to do that later.
| @@ -2507,7 +2507,8 @@ void File__Analyze::DisplayAspectRatio_Fill(const Ztring &Value, stream_t Stream | |||
| else if (DAR>=(float)2.15 && DAR<(float)2.22) DARS=__T("2.2:1"); | |||
| else if (DAR>=(float)2.23 && DAR<(float)2.30) DARS=__T("2.25:1"); | |||
| else if (DAR>=(float)2.30 && DAR<(float)2.37) DARS=__T("2.35:1"); | |||
| else if (DAR>=(float)2.37 && DAR<(float)2.45) DARS=__T("2.40:1"); | |||
| else if (DAR>=(float)2.37 && DAR<(float)2.40) DARS=__T("2.39:1"); | |||
| else if (DAR>=(float)2.40 && DAR<(float)2.45) DARS=__T("2.40:1"); | |||
There was a problem hiding this comment.
Should we really keep that 2.40:1 value?
There was a problem hiding this comment.
I dunno, it could break compatibility with some tools that are expecting that value? Though you could say the same for my commit, but I feel like this commit is closer to fixing a 'bug'. What is your reasoning for removing 2.40? That it is not a standardised ratio? I'd be in favour of keeping it ..
There was a problem hiding this comment.
Yes, because i'm always hesitant to leave not standardized stuff. Also if you are right that could break some program parsing code, as when we correct codec name or change the codec informations displayed, i consider that the tools, programs using MediaInfo have to adapt.
That said, in that case that doesn't hurt AFAIK and it is only one line of code so...
It is sane, sure, and IMO it is bug fix so "breaking" (it should break nothing as I expect people to use the raw vvalue) old stuff is fine. I let you both debate while I enjoy some days AFK :). |
…ng #612
This attempts to fix #612. I haven't tested this yet but I wanted to see if you thought this was sane.. It looks like two of us have a use case to have 2.39 display as intended anyhow..