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

Bugfix correctly load non square pixels for AVF player. Closes #4937 #4949

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ofTheo
Copy link
Member

@ofTheo ofTheo commented Feb 25, 2016

Before the AVF player was loading a non square video as ( for example ) 1920x1080 but both the pixel data and the texture coming from AVFoundation were actually 1440x1080.

This caused a crash when accessing pixels and resulted in the texture being drawn in a strange way.
see : https://forum.openframeworks.cc/t/issues-with-pixel-ratios-in-ofvideoplayer/22120/10?u=theo

This PR loads the video at the size that represent the real pixel data and adds the ability to query what the intended stretched display size is meant to be.

For now the new calls to getDisplayWidth() / getDisplayHeight are isolated to the ofAVFoundationPlayer class - if this is something we might want to support across platforms on linux and windows we could add it to the ofVideoPlayer and potentially even make the draw calls aware of the scaling needed.

For now this seems like a conservative fix which leaves the display scaling up to the user, but provides them with the information they need to account for the non square pixels.

An example of how you would do that is:

ofAVFoundationPlayer player;

//--------------------------------------------------------------
void ofApp::setup(){
    player.load("HDV_test.mov");
    player.play();
    ofSetWindowPosition(0, 0);
}

//--------------------------------------------------------------
void ofApp::update(){
    player.update();
}

//--------------------------------------------------------------
void ofApp::draw(){
    if( player.isAnamorphic() ){
        player.draw(0, 0, player.getDisplayWidth(), player.getDisplayHeight());
    }else{
        player.draw(0, 0);
    }
}

@ofTheo
Copy link
Member Author

ofTheo commented Feb 26, 2016

okay - finally not erring on travis ( must have tougher warning/error compile settings ).
the travis errors I see now seem unrelated.

should be good to go.

@ofTheo
Copy link
Member Author

ofTheo commented Mar 1, 2016

pinging @i-n-g-o @danoli3 - maybe take a quick look over before I merge this in?
It should be pretty clean but it affects iOS and OS X

@i-n-g-o
Copy link
Contributor

i-n-g-o commented Mar 13, 2016

hi theo.
missed that ping for a while. (changed my mail-filters, let's see if i get pings earlier now)

once you got a CMFormatDescriptionRef already you could get the trackDimensionts like this:

CGSize trackDimensions = CMVideoFormatDescriptionGetPresentationDimensions(formatDescription, false, false);

videoWidth = trackDimensions.width;
videoHeight = trackDimensions.height;

with the test-movie this gives: 1440 x 1080 pixels.
maybe better to use the CMs description than calculating with pixel-aspect?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants