[MPlayer-dev-eng] New stereo output mode for OpenGL video driver

[Wolfgang Draxinger]

On Fri, 23 Dec 2011 23:37:02 +0100
Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:

> On Fri, Dec 23, 2011 at 11:25:05PM +0100, Reimar Döffinger wrote:
> > On Sun, Dec 18, 2011 at 11:53:54PM +0100, Wolfgang Draxinger wrote:
> > > +    case GL_3D_ROWINTERLACE_ODD: 
> > 
> > Trailing space.

Oops. Sorry.

> > You shouldn't be using tabs in this code.

Replaced them with spaces (though personally I prefer tabs ;).

> > Also using the glAdjustAlignment function instead of
> > GL_UNPACK_ALIGNMENT directly is more consistent with the rest of
> > the code, even if it is a bit overkill.

glAdjustAlignment is a MPlayer helper function. By prefixing it with
'gl' it's technically violating the OpenGL namespace and may confuse
people who don't know OpenGL, possible making them try to look it up in
the OpenGL reference pages. So I strongly suggest to re-prefix the
OpenGL helpers to something like 'mpglh'.

Also I feel more comfortable using glPixelStorei directly instead of
some helper function. In fact I'm not even sure if that function should
be there at all. There are some interactions between
GL_[UN]PACK_ROW_LENGTH and GL_[UN]PACK_ALIGNMENT which may get obscured
by such a function.

> > However, by using uint64_t type for the arrays you should save some
> > typing and you should have no need to set this up at all, since any
> > value should work.
> 
> That last part is maybe a bad idea, due to mismatching types then.

It is a bad idea. Also what's that thing about saving typing? The
text is generated and done. Nobody has to retype it ever again.

> > Passing NULL to glPolygonStipple is not allowed by my understanding.

Indeed. A bit inconsistent if you ask me, as it is perfectly legal
for glTexImage. It's been a very long time since I used stippling in
another project, so that's the reason for this lapsus.

> > You should only need 2 and a way to swap left and right, that should
> > simplify this a good bit.
> 
> I implemented a way to swap left and right.
> The documentation in vo_gl.c and DOCS/man/en/mplayer.1 should be
> updated, too.

Indeed I added those additional modi just for the exchange of
left/right, due to MPlayer's lack of such at that time. Swapping of
course is the nicer method and should also be available as
control/hotkey.

I modified my patch according to your suggestions, but I'm still
waiting for that stereo swap feature to show up in SVN, so I can
incorporate those as well, before submitting.

On the topic stereo: So far MPlayer, well the OpenGL video output driver
only supports Side-By-Side stereo. However there are also Over-Under
and separate track stereo (the last being supported by Matroska for
quite a long time). I think it would make more sense to decode stereo
into separate textures instead of adjusting texture coordinates. While
adding Over-Under support is trivial, separate track support would
require to first build a composit texture image (well glTexSubImage2D
would do the trick). But using multiple textures had other advantages
as well. For example for accelerating things like Multi View Coding (3D
BluRay), where one effectively receives a "merged" picture and
"differences" to that.