Planar YUV support

[Benjamin Otte]

Hey,

Here's an update on my work on YUV planar support, also known as "what
we got from the gstreamer-cairo hackfest". I've reworked my experimental
code from back then to incorporate the things we agreed on. It is not
complete yet, but only misses details. So if you're a maintainer of the
libraries in question, now is a good time for a review and complaints or
issues with the general design of the code.

The code can be found in these 3 places:
  http://cgit.freedesktop.org/~company/cairo/log?h=planar
  http://cgit.freedesktop.org/~company/pixman/log?h=planar
  http://cgit.freedesktop.org/~company/gst-plugins-cairo
I constantly rebase and update it while I work on it, so that the final
merge contains a proper set of patches.

I'd like to merge this stuff to the respective master branches soon
after Cairo 1.10 is out, so that it gets enough testing and exposure
before it's contained in the next major Cairo release, which I hope will
happen before the end of September.

I'll now give a short description of what these patches do, what the
issues are and what's missing.

pixman:

The code adds a pixman_color_space_t enum that can be used to specify
the color space the values are in. Valid values so far are ARGB,
ARGB_UNMULTIPLIED, YCBCR_SD, YCBCR_HD and YCBCR_JPEG. Various new
pixman_format_code_t's were added to match the planar formats used in
YUV. Finally a new constructor for images was added:
pixman_image_t *
pixman_image_create_planar (pixman_format_code_t      format,
                            pixman_color_space_t      color_space,
                            int                       width,
                            int                       height,
                            unsigned int              num_planes,
                            uint32_t                **bits,
                            int                      *rowstrides);
This constructor combines the above features to add support for all the
image representations commonly used. So if you want to create a
pixman_image for a GdkPixbuf with alpha channel, you'd do:
uint32_t *bits = (uint32_t *) gdk_pixbuf_get_data (pixbuf);
int stride = gdk_pixbuf_get_rowstride (pixbuf);
pixman_image_create_planar (PIXMAN_r8g8b8a8,          
                            PIXMAN_COLOR_SPACE_ARGB_UNMULTIPLIED,
                            gdk_pixbuf_get_width (pixbuf),
                            gdk_pixbuf_get_height (pixbuf),
                            1,
                            &bits,
                            &stride);
Or to create an image from a GStreamer I420 buffer, you would do:
for (i = 0; i < 3; i++) {
  bits[i] = (uint32_t *) (buffer_data +
      gst_video_get_component_offset (GST_VIDEO_FORMAT_I420,
                                      i, width, height));
  strides[i] = gst_video_format_get_row_stride (GST_VIDEO_FORMAT_I420,
                                                i, width);
}
pixman_image_create_planar (PIXMAN_y420,
                            PIXMAN_COLOR_SPACE_YCBCR_SD,
                            width, height,
                            3, bits, strides);
That is roughly anything you need to know as a user of pixman.

Details missing in the implementation that I intend to fix:
- The conversion matrices for HD and JPEG color spaces are still
missing. The ones I randomly copied so far were all wrong. And I was too
lazy to do the math myself yet. (I blame COG for getting them wrong.)
- Writing for i420 is not implemented. I haven't found a way to
implement it yet that passed my not-ugly test.
- There is no fast paths at all for the YUV-related formats yet. I
certainly want to add the ones that are needed, but that requires some
real life tests first.
- Getting the fetchers right for subsampled formats and different
filters.
Things that I don't intend to fix before merging, but would be happy to
see others have a go at:
- formats I consider "unimportant" such as 4:1:1 video or the other
4:2:0 chroma varieties other than the MPEG1 one that GStreamer uses and
that I've implemented.


cairo:

The only change visible to users of Cairo will be the addition of the
cairo_color_space_t enum, additions to the cairo_format_t enum that
expose the required pixman formats and a new constructor:
cairo_public cairo_surface_t *
cairo_image_surface_create_planar (cairo_format_t       format,
                                   cairo_color_space_t  color_space,
                                   int                  width,
                                   int                  height,
                                   unsigned int         n_planes,
                                   char **              data,
                                   int *                strides);
This constructor matches the pixman constructor above. Even the examples
would look very similar. For the GdkPixbuf you'd do:
uint32_t *bits = (uint32_t *) gdk_pixbuf_get_data (pixbuf);
int stride = gdk_pixbuf_get_rowstride (pixbuf);
cairo_image_surface_create_planar (CAIRO_FORMAT_RGBA32,
                                   CAIRO_COLOR_SPACE_ARGB_UNMULTIPLIED,
                                   gdk_pixbuf_get_width (pixbuf),
                                   gdk_pixbuf_get_height (pixbuf),
                                   1,
                                   &bits,
                                   &stride);
In fact, I'm using a gdk patch right now that does exactly this. (I also
have a WebKit patch that does this to improve <canvas> and svg filters.)
To create an image from a GStreamer I420 buffer, you would do:
for (i = 0; i < 3; i++) {
  bits[i] = (uint32_t *) (buffer_data +
      gst_video_get_component_offset (GST_VIDEO_FORMAT_I420,
                                      i, width, height));
  strides[i] = gst_video_format_get_row_stride (GST_VIDEO_FORMAT_I420,
                                                i, width);
}
cairo_image_surface_create_planar (CAIRO_FORMAT_PLANAR_420,
                                   CAIRO_COLOR_SPACE_YCBCR_SD,
                                   width, height,
                                   3, bits, strides);
And that is exactly what gst-plugins-cairo does.

I actually had to write quite a bit of code in Cairo to make this work,
because a lot of the backends assume they get image surfaces in one of
the 3 common format: A8, RGB24 or ARGB32. And this of course is no
longer true.
To be sure to catch all the cases, I removed the image_surface's data
and stride members. With planar images, they don't make any sense
anyway. That of course causes a lot of compilation failures and I
haven't yet fixed all of them. In particular the backends that don't
work on my computer (Most notably win32 and CoreGraphics) or
experimentaql haven't been fixed.

Another thing I haven't decided on and need some input is what
backend(s) to focus on for accelerated uploads (and downlaods) of YUV
image data. The obvious choices are GL and XCB, but GL is still
experimental and XCB suffers from X not having any support for YUV (no,
xv doesn't count). (Before anyone asks: In the long run both should be
well supported, but I still need something to start with).

gst-plugins-cairo:

There are no real changes to the API since the hackfest. I just updated
the internals to conform to the new API and thread-safety guarantees
from Cairo. (And that was mostly deleting code).
My first goal here is to make the gstreamer sink the default videosink.
And after that work on improved embedding of video streams (think
browsers, but also Clutter) and a live editing framework for video
editors like Pitivi. (annotating Byzanz screencasts should be trivial!)
What I've been wondering about though is what subset of currently
supported YUV/RGB formats is a sane subset. It doesn't make a lot of
sense to me to support all those crazy formats when no one uses them.
I'm in particular thinking about Y41B, NV21, and all those weird RGB
formats. Can we agree on a list of formats that we want to support
"natively" as opposed to "convert asap to a native format"?


So, that's the current state of the gstreamer-cairo saga, now it's
Søren's, Carl's, Chris' and others turn to comment on it if they wanna
avoid me merging it as is. ;)

Cheers,
Benjamin


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel

[Arjen Nienhuis]

Hi,

Your works looks really useful! Did you make read/write support for
all these formats? That means it can speedup things like JPEG
enc/decoding right?

Minor nitpick:

I think pixman_color_space_t is not the right name. It's ambiguous
because color space can also mean sRGB vs AdobeRGB. What about
'encoding'?


On Tue, Mar 9, 2010 at 5:09 PM, Benjamin Otte <[hidden email]> wrote:

> Hey,
>
> Here's an update on my work on YUV planar support, also known as "what
> we got from the gstreamer-cairo hackfest". I've reworked my experimental
> code from back then to incorporate the things we agreed on. It is not
> complete yet, but only misses details. So if you're a maintainer of the
> libraries in question, now is a good time for a review and complaints or
> issues with the general design of the code.
>
> The code can be found in these 3 places:
>  http://cgit.freedesktop.org/~company/cairo/log?h=planar
>  http://cgit.freedesktop.org/~company/pixman/log?h=planar
>  http://cgit.freedesktop.org/~company/gst-plugins-cairo
> I constantly rebase and update it while I work on it, so that the final
> merge contains a proper set of patches.
>
> I'd like to merge this stuff to the respective master branches soon
> after Cairo 1.10 is out, so that it gets enough testing and exposure
> before it's contained in the next major Cairo release, which I hope will
> happen before the end of September.
>
> I'll now give a short description of what these patches do, what the
> issues are and what's missing.
>
> pixman:
>
> The code adds a pixman_color_space_t enum that can be used to specify
> the color space the values are in. Valid values so far are ARGB,
> ARGB_UNMULTIPLIED, YCBCR_SD, YCBCR_HD and YCBCR_JPEG. Various new
> pixman_format_code_t's were added to match the planar formats used in
> YUV. Finally a new constructor for images was added:
> pixman_image_t *
> pixman_image_create_planar (pixman_format_code_t      format,
>                            pixman_color_space_t      color_space,
>                            int                       width,
>                            int                       height,
>                            unsigned int              num_planes,
>                            uint32_t                **bits,
>                            int                      *rowstrides);
> This constructor combines the above features to add support for all the
> image representations commonly used. So if you want to create a
> pixman_image for a GdkPixbuf with alpha channel, you'd do:
> uint32_t *bits = (uint32_t *) gdk_pixbuf_get_data (pixbuf);
> int stride = gdk_pixbuf_get_rowstride (pixbuf);
> pixman_image_create_planar (PIXMAN_r8g8b8a8,
>                            PIXMAN_COLOR_SPACE_ARGB_UNMULTIPLIED,
>                            gdk_pixbuf_get_width (pixbuf),
>                            gdk_pixbuf_get_height (pixbuf),
>                            1,
>                            &bits,
>                            &stride);
> Or to create an image from a GStreamer I420 buffer, you would do:
> for (i = 0; i < 3; i++) {
>  bits[i] = (uint32_t *) (buffer_data +
>      gst_video_get_component_offset (GST_VIDEO_FORMAT_I420,
>                                      i, width, height));
>  strides[i] = gst_video_format_get_row_stride (GST_VIDEO_FORMAT_I420,
>                                                i, width);
> }
> pixman_image_create_planar (PIXMAN_y420,
>                            PIXMAN_COLOR_SPACE_YCBCR_SD,
>                            width, height,
>                            3, bits, strides);
> That is roughly anything you need to know as a user of pixman.
>
> Details missing in the implementation that I intend to fix:
> - The conversion matrices for HD and JPEG color spaces are still
> missing. The ones I randomly copied so far were all wrong. And I was too
> lazy to do the math myself yet. (I blame COG for getting them wrong.)
> - Writing for i420 is not implemented. I haven't found a way to
> implement it yet that passed my not-ugly test.
> - There is no fast paths at all for the YUV-related formats yet. I
> certainly want to add the ones that are needed, but that requires some
> real life tests first.
> - Getting the fetchers right for subsampled formats and different
> filters.
> Things that I don't intend to fix before merging, but would be happy to
> see others have a go at:
> - formats I consider "unimportant" such as 4:1:1 video or the other
> 4:2:0 chroma varieties other than the MPEG1 one that GStreamer uses and
> that I've implemented.
>
>
> cairo:
>
> The only change visible to users of Cairo will be the addition of the
> cairo_color_space_t enum, additions to the cairo_format_t enum that
> expose the required pixman formats and a new constructor:
> cairo_public cairo_surface_t *
> cairo_image_surface_create_planar (cairo_format_t       format,
>                                   cairo_color_space_t  color_space,
>                                   int                  width,
>                                   int                  height,
>                                   unsigned int         n_planes,
>                                   char **              data,
>                                   int *                strides);
> This constructor matches the pixman constructor above. Even the examples
> would look very similar. For the GdkPixbuf you'd do:
> uint32_t *bits = (uint32_t *) gdk_pixbuf_get_data (pixbuf);
> int stride = gdk_pixbuf_get_rowstride (pixbuf);
> cairo_image_surface_create_planar (CAIRO_FORMAT_RGBA32,
>                                   CAIRO_COLOR_SPACE_ARGB_UNMULTIPLIED,
>                                   gdk_pixbuf_get_width (pixbuf),
>                                   gdk_pixbuf_get_height (pixbuf),
>                                   1,
>                                   &bits,
>                                   &stride);
> In fact, I'm using a gdk patch right now that does exactly this. (I also
> have a WebKit patch that does this to improve <canvas> and svg filters.)
> To create an image from a GStreamer I420 buffer, you would do:
> for (i = 0; i < 3; i++) {
>  bits[i] = (uint32_t *) (buffer_data +
>      gst_video_get_component_offset (GST_VIDEO_FORMAT_I420,
>                                      i, width, height));
>  strides[i] = gst_video_format_get_row_stride (GST_VIDEO_FORMAT_I420,
>                                                i, width);
> }
> cairo_image_surface_create_planar (CAIRO_FORMAT_PLANAR_420,
>                                   CAIRO_COLOR_SPACE_YCBCR_SD,
>                                   width, height,
>                                   3, bits, strides);
> And that is exactly what gst-plugins-cairo does.
>
> I actually had to write quite a bit of code in Cairo to make this work,
> because a lot of the backends assume they get image surfaces in one of
> the 3 common format: A8, RGB24 or ARGB32. And this of course is no
> longer true.
> To be sure to catch all the cases, I removed the image_surface's data
> and stride members. With planar images, they don't make any sense
> anyway. That of course causes a lot of compilation failures and I
> haven't yet fixed all of them. In particular the backends that don't
> work on my computer (Most notably win32 and CoreGraphics) or
> experimentaql haven't been fixed.
>
> Another thing I haven't decided on and need some input is what
> backend(s) to focus on for accelerated uploads (and downlaods) of YUV
> image data. The obvious choices are GL and XCB, but GL is still
> experimental and XCB suffers from X not having any support for YUV (no,
> xv doesn't count). (Before anyone asks: In the long run both should be
> well supported, but I still need something to start with).
>
> gst-plugins-cairo:
>
> There are no real changes to the API since the hackfest. I just updated
> the internals to conform to the new API and thread-safety guarantees
> from Cairo. (And that was mostly deleting code).
> My first goal here is to make the gstreamer sink the default videosink.
> And after that work on improved embedding of video streams (think
> browsers, but also Clutter) and a live editing framework for video
> editors like Pitivi. (annotating Byzanz screencasts should be trivial!)
> What I've been wondering about though is what subset of currently
> supported YUV/RGB formats is a sane subset. It doesn't make a lot of
> sense to me to support all those crazy formats when no one uses them.
> I'm in particular thinking about Y41B, NV21, and all those weird RGB
> formats. Can we agree on a list of formats that we want to support
> "natively" as opposed to "convert asap to a native format"?
>
>
> So, that's the current state of the gstreamer-cairo saga, now it's
> Søren's, Carl's, Chris' and others turn to comment on it if they wanna
> avoid me merging it as is. ;)
>
> Cheers,
> Benjamin
>
> --
> cairo mailing list
> [hidden email]
> http://lists.cairographics.org/mailman/listinfo/cairo

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel

[michael smith-6-3]

On Tue, Mar 9, 2010 at 9:00 AM, Arjen Nienhuis <[hidden email]> wrote:

> Hi,
>
> Your works looks really useful! Did you make read/write support for
> all these formats? That means it can speedup things like JPEG
> enc/decoding right?
>
> Minor nitpick:
>
> I think pixman_color_space_t is not the right name. It's ambiguous
> because color space can also mean sRGB vs AdobeRGB. What about
> 'encoding'?

pixel_format would probably be a better name if it's _only_ intended
to encode the pixel format. I think Benjamin's intent, though, was to
encode a pair of "pixel format, colour space" in a single value.
That's why his suggestion has YCBCR_SD and YCBCR_HD - these have the
same pixel formats, but imply a different colour space.

That's my understanding anyway - hopefully Benjamin can clarify if I
got anything wrong there.

Mike

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel

[Benjamin Otte]

On Tue, 2010-03-09 at 10:36 -0800, Michael Smith wrote:

> On Tue, Mar 9, 2010 at 9:00 AM, Arjen Nienhuis <[hidden email]> wrote:
> > I think pixman_color_space_t is not the right name. It's ambiguous
> > because color space can also mean sRGB vs AdobeRGB. What about
> > 'encoding'?
>
> pixel_format would probably be a better name if it's _only_ intended
> to encode the pixel format. I think Benjamin's intent, though, was to
> encode a pair of "pixel format, colour space" in a single value.
> That's why his suggestion has YCBCR_SD and YCBCR_HD - these have the
> same pixel formats, but imply a different colour space.
>

I've started to think about image data as an N-dimensional cube. N
denotes the number of channels in the image, so for RGB it's 3, for grey
it would be 1.
There's various additional informations about that, like how many
different values there are per dimension (256 in most cases these days
for RGB32) and how they are put into the memory of your computer (This
is what RGB vs BGR or planar vs packed is all about). This is encoded in
pixman with the pixman_format_code_t enum.

There's a separate question to all of this and that question concerns
itself with what color one such value actually represents. So far in
RGB-land, people were content with claiming that (0, 0, 0) is "black"
and (max, max, max) is "white". However, this isn't true anymore once
you want to support YUV - where (0, 0, 0) is dark green. So I used the
pixman_color_space_t to encode this meaning for my purposes.
There have been a lot of proponents for making this a universal anything
to encode anything from sRGB vs AdobeRGB to ICC profiles and spot colors
with this value, but as I said in a separate mail already: So far noone
wrote any code yet.

Cheers,
Benjamin


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel

[Ville Syrjälä]

On Tue, Mar 09, 2010 at 10:36:13AM -0800, Michael Smith wrote:

> On Tue, Mar 9, 2010 at 9:00 AM, Arjen Nienhuis <[hidden email]> wrote:
> > Hi,
> >
> > Your works looks really useful! Did you make read/write support for
> > all these formats? That means it can speedup things like JPEG
> > enc/decoding right?
> >
> > Minor nitpick:
> >
> > I think pixman_color_space_t is not the right name. It's ambiguous
> > because color space can also mean sRGB vs AdobeRGB. What about
> > 'encoding'?
>
> pixel_format would probably be a better name if it's _only_ intended
> to encode the pixel format. I think Benjamin's intent, though, was to
> encode a pair of "pixel format, colour space" in a single value.
> That's why his suggestion has YCBCR_SD and YCBCR_HD - these have the
> same pixel formats, but imply a different colour space.

It's also a bit unclear what range these use. I'm assuming the
idea was that YCBCR_{SD,HD} use the 16-235 range and YCBCR_JPEG the
0-255 range.

One thing that's completely missing is the chroma siting. For 420P
there are three different variations in use. Four if you count the
MPEG2 interlaced version as a separate thing.

I don't know how fancy people want to go with this stuff but ffmpeg
and microsoft have some relevant stuff here:
http://git.ffmpeg.org/?p=ffmpeg;a=blob;f=libavcodec/avcodec.h
http://msdn.microsoft.com/en-us/library/bb970322(VS.85).aspx

--
Ville Syrjälä
[hidden email]
http://www.sci.fi/~syrjala/

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel

[Kai-Uwe Behrmann]

Am 10.03.10, 11:03 +0100 schrieb Benjamin Otte:

> On Tue, 2010-03-09 at 10:36 -0800, Michael Smith wrote:
>> On Tue, Mar 9, 2010 at 9:00 AM, Arjen Nienhuis <[hidden email]> wrote:
>>> I think pixman_color_space_t is not the right name. It's ambiguous
>>> because color space can also mean sRGB vs AdobeRGB. What about
>>> 'encoding'?
>>
>> pixel_format would probably be a better name if it's _only_ intended
>> to encode the pixel format. I think Benjamin's intent, though, was to
>> encode a pair of "pixel format, colour space" in a single value.
>> That's why his suggestion has YCBCR_SD and YCBCR_HD - these have the
>> same pixel formats, but imply a different colour space.
>>
> I've started to think about image data as an N-dimensional cube. N
> denotes the number of channels in the image, so for RGB it's 3, for grey
> it would be 1.
> There's various additional informations about that, like how many
> different values there are per dimension (256 in most cases these days
> for RGB32) and how they are put into the memory of your computer (This
> is what RGB vs BGR or planar vs packed is all about). This is encoded in
> pixman with the pixman_format_code_t enum.
>
> There's a separate question to all of this and that question concerns
> itself with what color one such value actually represents. So far in
> RGB-land, people were content with claiming that (0, 0, 0) is "black"
> and (max, max, max) is "white". However, this isn't true anymore once
> you want to support YUV - where (0, 0, 0) is dark green. So I used the
> pixman_color_space_t to encode this meaning for my purposes.
> There have been a lot of proponents for making this a universal anything
> to encode anything from sRGB vs AdobeRGB to ICC profiles and spot colors
> with this value, but as I said in a separate mail already: So far noone
> wrote any code yet.

You mean you did not write a pixman_color_space_t type. Given that there
is repreatedly interesst in a qualified colour API in Cairo, it would
be quite useful not to enter the term "color space" for some fixed meaning
enums in opposite to a generic flexible system.

Btw colour characteristics of BT.601, BT.709 and friends can be decribed
with ICC profiles.


kind regards
Kai-Uwe Behrmann
--
developing for colour management
www.behrmann.name + www.oyranos.org


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel

[Soeren Sandmann-2]

Hi Benjamin,

> Here's an update on my work on YUV planar support, also known as "what
> we got from the gstreamer-cairo hackfest". I've reworked my experimental
> code from back then to incorporate the things we agreed on. It is not
> complete yet, but only misses details. So if you're a maintainer of the
> libraries in question, now is a good time for a review and complaints or
> issues with the general design of the code.

First of all, thanks for working on YCrCb support. There is a lot of
interesting things that become possible with this. If all goes as
planned, this will be the major new feature in 0.20.0.

> Details missing in the implementation that I intend to fix:
> - Getting the fetchers right for subsampled formats and different
> filters.

This has to do with an overall concern I have, which is about how
YCrCb fits in with pixman's image processing pipeline, and how to do
filtering and subsample locations.

The existing 8 bits R'G'B' pipeline looks like this:

     * Convert each image sample to a8r8g8b8
     * Extend grid in all directions according to repeat mode
     * Interpolate between samples according to filter
     * Transform
     * Resample
     * Combine
     * Store

Your patches add YCbCr support at the first stage in the sense that it
converts each Y sample to a8r8g8b8 using the nearest neighbor Cb and
Cr samples.

The problem with this scheme is that if we add bilinear interpolation
and support for MPEG-2 sampling structure, then those features would
also have to be added at the fetching stage. And then the later
interpolation stage would either do the wrong thing by interpolating
_again_, or it would have to be disabled and the fetch stage would
have to deal with fetching with transformed coordinates. Basically,
this gets really messy quickly.

Instead, we need to make some changes to the pipeline. A YCbCr image
is an RGB image that has had two things done to it:

     1. It was converted to YCbCr
     2. It had its Cb and Cr components subsampled

To get such an image back to RGB where we can composite with it, we
have to reverse each of those transformations: First we have to
reverse the subsampling, then we have to reverse the color coding.

Reversing subsampling is a form of interpolation, so it seems natural
to do it as part of the existing interpolation step. Since color
conversion has to happen after interpolation, this then implies that
the first stage can no longer be "convert to argb", but instead must
simply be "widen to 8 bits, while keeping the existing color coding".

The location of the chroma sample points varies from format to
format. This means that the interpolation code will have to apply
special adjustments when it computes which samples to use for any
given intermediate point.

The new pipeline then looks like this:

      * Widen to 8 bit components
      * Extend
      * Interpolate between samples according to filter
      * Transform
      * Convert to RGB coding
      * Resample
      * Combine
      * Store

The PIXMAN_COLOR_SPACE_YCBCR_* entries in the pixman_color_space_t
enum in the branch would be used in the 'Convert to RGB coding' stage.

But the PIXMAN_COLOR_SPACE_ARGB_UNMULTIPLIED doesn't fit in here
because premultiplication is something that has to happen _before_
interpolation, whereas color decoding needs to happen after. This
suggests to me that those two things don't belong in the same enum. I
do think support for unpremultiplied formats is worthwhile, but it
seems orthogonal to YCrCb support.

In practical terms, the above means YCrCb processing will have to go
through the bits_image_fetch_transformed() path and that the
fetch_scanline() and fetch_pixel() function pointers should be
considered *sample* fetchers that simply widen and complete the
samples wrt. their existing color coding, but doesn't try to do
interpolation or color conversion. The x and y coordinates passed to
them must then always be integers and refer to samples. If you pass
(1, 1) to one of those fetchers, the result will be the second sample
on the second row of the component in question.

It is then the job of the code in fetch_transformed() to ask the
fetchers for the correct samples for a given pair of (fractional)
coordinates, and then interpolate between those samples.

Since the color doing will have to be preserved all the way up to the
fetch_transformed() stage, we will need separate fetchers for the Y,
Cb and Cr components. The transforming code path will have to figure
out which fetchers to use.

In principle we could have separate A, R, G, B fetchers as well, but
we can leave that alone for now since we don't have subsampled argb
formats.

So the way I'd write this code is to add a new enum

    COMPONENT_ARGB
    COMPONENT_Y
    COMPONENT_CB
    COMPONENT_CR

in pixman-bits-image.c and then extend the various
fetch_pixel_<filter> to take that enum as an argument.

The bits_image_fetch_pixel_filtered() function then is extended to
check the color coding for the image in question and if it is YCbCr,
fetch the three components separately and then do the color
conversion. Ie.,

    if (is_ycbcr)
    {
        x_Y, y_Y = x, y;

        x_Cb, y_Cb = (adjust for subsampling structure);
        x_Cr, y_Cr = (adjust for subsampling structure);
    }

    switch (image->common.filter)
    {
    case PIXMAN_FILTER_NEAREST:
    case PIXMAN_FILTER_FAST:
        if (is ycbcr)
        {
            y = fetch_nearest (COMPONENT_Y, x, y);
            cb = fetch_nearest (COMPONENT_CB, x_Cb, y_Cb)
            cr = fetch_nearest (COMPONENT_CR, x_Cr, y_Cr);

            return color_convert (image, y, cb, cr);
        }
        else
        {
            return fetch_nearest (image, COMPONENT_ARGB, x, y);
        }
        break;

        ...;
    }

where get_pixel() then finally picks the right fetcher to use
based on the enum value.

Maybe your idea of eliminating the get_pixel() altogether and just use
fetch_scanline() with a length of one could make this simpler.


Thanks,
Soren

------------------------------------------------------------------------------

_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel