[FFmpeg-devel] [PATCH 3/3] lavd: add opengl device

[Michael Niedermayer]

On Thu, Jan 23, 2014 at 11:45:02PM +0100, Lukasz Marek wrote:
> On 19.01.2014 17:35, Lukasz M wrote:
> >On 19 January 2014 14:46, Lukasz M <lukasz.m.luki at gmail.com
> ><mailto:lukasz.m.luki at gmail.com>> wrote:
> >
> >    On 19 January 2014 03:46, Lukasz M <lukasz.m.luki at gmail.com
> >    <mailto:lukasz.m.luki at gmail.com>> wrote:
> >
> >        Updated patch attached. I hope I handled every remark.
> >
> >        Tested on
> >        SDL window: Windows (mingw), Ubuntu 12.04, Ubuntu 13.10, Mac 10.7
> >        External context: qt 4.8 app on Ubuntu 13.10, iOS ipad
> >        simulator, iPad 2 device.
> >
> >        I will submit examples later, but I wanted to use
> >        write_uncoded_frame, so examples are more focused on device, not
> >        on demuxing, muxing etc...
> >
> >
> >      libavdevice/avdevice.h           |    2 +-
> >      libavformat/avformat.h           |    2 +-
> >
> >    I just noticed I squashed things to wrong commit, I will resubmit it
> >    with these changes removed.
> >
> >
> >patch rebased on newer control message API and removed 2 changes
> >mentioned above.
> >
> 
> Another rebase. Any comments btw?

about opengl itself, not from me, iam no opengl expert
others around here know opengl much better ...


[...]

> +#if defined(__APPLE__)
> +#if HAVE_OPENGL_GL3_H
> +#include <OpenGL/gl3.h>
> +#else
> +#include <OpenGLES/ES2/gl.h>
> +#endif
> +#else
> +#include <GL/gl.h>
> +#include <GL/glext.h>
> +#endif
> +
> +#if HAVE_SDL
> +#include <SDL.h>
> +#endif
> +#if HAVE_GLXGETPROCADDRESS
> +#include <GL/glx.h>
> +#endif
> +#if defined(__MINGW32__)
> +#include <windows.h>
> +#endif
> +

these checks on __APPLE__ and __MINGW32__ smell wrong
code should generally not check on platform type but on the actual
things that you need like header availability or whatever



[...9

> +#define BUFFER_OFFSET(i) ((char *)NULL + (i))

iam not sure this is strictly allowed in C99

[...]

> +static av_cold void opengl_get_texture_params(OpenGLContext *opengl)
> +{
> +    switch(opengl->pix_fmt) {
> +    case AV_PIX_FMT_YUV420P:    case AV_PIX_FMT_YUV444P:
> +    case AV_PIX_FMT_YUV422P:    case AV_PIX_FMT_YUV410P:
> +    case AV_PIX_FMT_YUV411P:    case AV_PIX_FMT_YUV440P:
> +    case AV_PIX_FMT_YUVA420P:   case AV_PIX_FMT_YUVA444P:
> +    case AV_PIX_FMT_YUVA422P:
> +    case AV_PIX_FMT_GBRP:       case AV_PIX_FMT_GBRAP:
> +#if defined(GL_RED)
> +        opengl->format = GL_RED;
> +#elif defined(GL_LUMINANCE)
> +        opengl->format = GL_LUMINANCE;
> +#else
> +        opengl->format = 0x1903; //GL_RED
> +#endif

why is this handled differently than for example a missing M_PI ?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140124/bc817ac6/attachment.asc>