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

Paul B Mahol onemda at gmail.com
Thu Jan 24 16:43:09 CET 2013


On 1/24/13, Michael Bradshaw <mjbshaw at gmail.com> wrote:
> 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].

While you are at it, why dont you make use of memcpy() when you can, it
should be magnitude faster.

>
> [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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list