Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix VideoResolution infolabel for 1088p. #6391

Closed
wants to merge 1 commit into from

Conversation

anaconda
Copy link
Contributor

@anaconda anaconda commented Feb 9, 2015

It seems 1088p videos exist. We report 1080 for 1088p, like we do already
for 544p -> 540.

See:

It seems 1088p videos exist.  We report 1080 for 1088p, like we do already
for 544p -> 540.

See:
  * http://forum.kodi.tv/showthread.php?tid=217739
  * http://forum.kodi.tv/showthread.php?tid=199889
@@ -557,8 +557,8 @@ std::string CStreamDetails::VideoDimsToResolutionDescription(int iWidth, int iHe
// 1280x720
else if (iWidth <= 1280 && iHeight <= 720)
return "720";
// 1920x1080
else if (iWidth <= 1920 && iHeight <= 1080)
// 1920x1080 (sometimes 1088, which is a multiple of 16 - see comment about 544)

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@Jalle19
Copy link
Member

Jalle19 commented Feb 10, 2015

I tried to fix this once but got shot down since it was a tvheadend "bug". Maybe some parser is doing the wrong thing somewhere this time too?

@fritsch
Copy link
Member

fritsch commented Feb 10, 2015

"MPEG2 and MPEG4 require frame sizes to be multiples of 16 pixels because macroblocks are 16x16 pixels" Every MPEG2 video ever encoded as 1080 actually has 1088 lines encoded because 1080 is not evenly divided by 16. The industry standard is to set the last 8 lines of the frame to black. Then, the display device is expected to throw away the last 8 pixels and not display them"

@popcornmix
Copy link
Member

Yes, most codecs encode 1080p as 1920x1088, but they also provide a crop rectangle of 1920x1080 (e.g. H.264 has frame_cropping_flag). Often when the height is reported as 1088, it is because the crop rectangle hasn't been parsed and that is just a bug.

It is obviously possible to have a genuine 1920x1088 video, but that's not ideal for displaying on a 1080p display, so I'd be surprised if they were very common. They probably occur when the user encoding the video knows a little bit about macroblock sizes and (wrongly) thinks multiples of 16 are required.

@anaconda
Copy link
Contributor Author

@FernetMenta do you still want a sample (after the last 2 comments)? Because I don't have any myself. I guess the interwebs can give me some though.
Before opening this PR I have asked Google and I was able to find some complaining about their cameras giving them a 1920x1088 video.

@FernetMenta
Copy link
Contributor

yes, if it is not too much effort to get one

@anaconda
Copy link
Contributor Author

Finally found one: https://menakite.eu/~anaconda/XMenIII.mp4

Edit: also https://www.dropbox.com/s/8j9b6kn7s4y5l7a/XMenIII.mp4?dl=0
(pretty crappy, but 1920x1088)

@FernetMenta
Copy link
Contributor

Thanks! I think that is a result of user error on encoding. I still don't think that we should silently ignore this problem by displaying a 1080 label. We can display something else if it's worth the effort.

@MartijnKaijser
Copy link
Member

for proper handling see #6680

@anaconda anaconda deleted the infolabel-videores-1088 branch August 16, 2015 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants