descampsa / yuv2rgb

C99 library for fast image conversion between yuv420p and rgb24
BSD 3-Clause "New" or "Revised" License
170 stars 62 forks source link

Integrate yuv420tobgr24 into FFMPEG #11

Closed ghost closed 2 years ago

ghost commented 5 years ago

I would like to use this implementation in place of the MMX version currently used in ffmpeg. I compile ffmpeg myself and would like to know the easiest way to integrate this.

Also, it would be great if you can upstream this to ffmpeg.

ghost commented 5 years ago

cc @descampsa

descampsa commented 5 years ago

Hi,

What do you want to achieve exactly? If you only want to replace a call to sw_scale that convert between rgb and yuv, you don't need to integrate this code to ffmpeg, you just have have to call the appropriate function with the correct argument. I can give you an example if you provides your current code.

If you really want this library to be integrated to ffmpeg, that is possible but would require more work.

ghost commented 5 years ago

Hi @descampsa

Thanks for the reply. I have a workload which spends ~10% of the time in yuv420_bgr24_mmxext in libswscale.so.

I am trying to use your implementation which shows significant improvement on my SandyBridge machine. (And I need to implement BGR, instead of RGB. I hope this is going to be a small change).

yuv420_bgr24_mmxext is implemented here. https://github.com/FFmpeg/FFmpeg/blob/master/libswscale/x86/yuv2rgb_template.c#L337 and it has a function signature of following

static inline int RENAME(yuv420_bgr24)(SwsContext c, const uint8_t src[], int srcStride[], int srcSliceY, int srcSliceH, uint8_t *dst[], int dstStride[])

And your function has the following signature.

void yuv420_rgb24_sseu( uint32_t width, uint32_t height, const uint8_t Y, const uint8_t U, const uint8_t V, uint32_t Y_stride, uint32_t UV_stride, uint8_t RGB, uint32_t RGB_stride, YCbCrType yuv_type)

I am looking at the parameters, I think I can modify your function with the following parameter mappings. (I do not know where to get the YCbCrType, also I do not what this srcSliceY is, you seem to pass a 0 to it).

width=c->dstW height=srcSliceH Y_stride=srcStride[0] UV_stride=srcStride[1] RGB=dst RGB_stride=dstStride[0]

Thanks -Adrian

descampsa commented 5 years ago

And I need to implement BGR, instead of RGB. I hope this is going to be a small change.

All you have to do is invert the r and b parameters in the calls to the PACK_RGB24_32 macro.

I am looking at the parameters, I think I can modify your function with the following parameter mappings. (I do not know where to get the YCbCrType, also I do not what this srcSliceY is, you seem to pass a 0 to it). width=c->dstW height=srcSliceH Y_stride=srcStride[0] UV_stride=srcStride[1] RGB=dst RGB_stride=dstStride[0]

That looks ok, except RGB shoud be dst[0], and Y=src[0], U=src[1] and V=src[2]. For the YCbCrType, it is not simple to get it from the context, since ffmpeg apparently computes the conversion factor at context creation, but does not store the color space itself. Actually, i am not sure which function in ffmpeg is used to select the YUV colorspace, since sws_getContext seems to always use SWS_CS_DEFAULT, that is SWS_CS_ITU601. Unless you know you need something else, i would simply set YCbCrType to YCBCR_601.

srcSliceY is the first row to use in the image, it should always be 0 for complete image conversion. That is probably not needed in most cases, but you can handle non zero values by shifting the input and output pointers by srcSliceY multiplied by the appropriate stride value.

Y = src[0] + srcSliceY * srcStride[0]; U = src[1] + srcSliceY * srcStride[1]; V = src[2] + srcSliceY * srcStride[1]; RGB = dst[0] + srcSliceY * dstStride[0];

Once you have a function with the correct signature, you will have to modify the ff_yuv2rgb_init_x86 function to return a pointer to your function for yuv420 to bgr24 conversion.

ghost commented 5 years ago

@descampsa Thanks for the reply !. I got it working mostly. I see the resulted image is slightly different from the one produced by ffmpeg. Do you know where the source of variation is ?

I mean if I run the tests, i get .ppm files that differ. They look identical, but diff FILE1.ppm FILE2.ppm reveals differences.

Thanks

descampsa commented 5 years ago

It does not surprise me that a diff reveals differences, since the algorithm are not exactly the same, there will be differences in result due to rounding errors. These differences should be small. You can use the compare tool from image magick to analyse the differences between images with some tolerance.

If the differences are important, i see two possible causes: 1) You do not use the same yuv colorspace than with ffmpeg. 2) If the width of the image is not divisible by 32, the last pixels will not be filled by my code.