[FFmpeg-devel] [PATCH] libopenjpegdec: add pix fmt gbrp (9-14 bit)

[Michael Bradshaw]

On Thu, Jan 24, 2013 at 7:42 AM, Paul B Mahol <onemda at gmail.com> wrote:

> On 1/24/13, Michael Bradshaw <mjbshaw at gmail.com> wrote:
> > On Thu, Jan 24, 2013 at 5:12 AM, Paul B Mahol <onemda at gmail.com> wrote:
> >
> >> On 1/24/13, Carl Eugen Hoyos <cehoyos at ag.or.at> wrote:
> >> > Paul B Mahol <onemda <at> gmail.com> writes:
> >> >
> >> >> Did you ever notice that RGB48 case is pure hack
> >> >> because you manully deinterleave/planarize RGB48.
> >> >
> >> > (Apart from the fact that as you know there is no
> >> > complete planar GBR support making the discussion
> >> > not really helpful:)
> >> > Wasn't that implemented before non-8 bit GBR
> >> > code was added or do I miss something?
> >> >
> >> >> Obviously you do not care for performance.
> >> >
> >> > When I last tested libopenjpeg, it was a magnitude
> >> > slower than our native codec, so I don't understand
> >> > how a performance argument for this wrapper is
> >> > relevant.
> >>
> >> If time spent in wrapper can be made smaller
> >> it is better.
> >>
> >> Yes, GBRP case can take priority over RGB case once
> >> GBRP gets output in swscale. Currently it is still possible to output
> >> both via user request.
> >
> >
> > Yes, this works (without this patch, which is one reason why this patch
> is
> > so useless). Thanks for your time, Paul.
>
> You missunderstood. Patch is not useless.
> The gbrp<->gbrp cases are still possible which without this patch are
> wasted in deinterleaving. I expected you would notice that adding option
> to decoder to output to gbrp instead to rgb is proper addition to patch
> itself.


Hmmm, I suppose you're right. The benefit of this patch doesn't come from
avoiding deinterleaving, though[1]. The benefit of this patch comes from
correctly creating gbrp frames (currently, the components are mixed up
since it's naively copying openjpeg's rgbp to ffmpeg's gbrp)[2].

[1] because deinterleaving is already avoided currently if the user
specifies a gbrp format; additionally, if the user does not specify a gbrp
format, and openjpeg can't determine the pixel format (and so far I have
not been able to get openjpeg to correctly determine the pixel format),
this patch does nothing since rgb and yuv formats have higher precedence in
the decoder.

[2] but again, as noted in [1], I can't get openjpeg to correctly identify
pixel formats reliably for the relevant formats, so the decoder defaults to
rgb or yuv, which completely negates this benefit.

I think this patch needs some serious revising in order for it to actually
be useful. Suggestions (or alternative patches) are welcome.

Thanks,

Michael