From 6834acd8779dbf5224a294c22a6d5f69f1903d3e Mon Sep 17 00:00:00 2001 From: Dan Dennedy Date: Mon, 20 Jan 2014 22:34:50 -0800 Subject: [PATCH] Fix a few problems with YCbCr colorspace conversion. In avformat producer on libav (and FFmpeg < v2.1) conversion from RGB to YCbCr would not use the destination colorspace because sws_getColorspaceDetails() fails. Switch to calling only sws_setColorspaceDetails(). In full luma yuvj420p->mlt_image_yuv420p conversion, the luma range was always scaled down to MPEG range. The swscale implementation does not let one override the range as the conversion routines are initialized at the time a swscale context is allocated and initialized. Any changes in sws_setColorspaceChanges() are mute. In RGB->YCbCr conversion, the existing (source) colorspace was used instead of the profile colorspace. Also, we need to set the new colorspace as a property of the frame. --- src/modules/avformat/filter_avcolour_space.c | 101 +++++++++------- src/modules/avformat/producer_avformat.c | 117 ++++++++++++------- 2 files changed, 134 insertions(+), 84 deletions(-) diff --git a/src/modules/avformat/filter_avcolour_space.c b/src/modules/avformat/filter_avcolour_space.c index 2e5fc624..627a3c82 100644 --- a/src/modules/avformat/filter_avcolour_space.c +++ b/src/modules/avformat/filter_avcolour_space.c @@ -22,6 +22,7 @@ #include #include #include +#include // ffmpeg Header files #include @@ -68,48 +69,60 @@ static int convert_mlt_to_av_cs( mlt_image_format format ) return value; } -static void set_luma_transfer( struct SwsContext *context, int colorspace, int use_full_range ) +static int set_luma_transfer( struct SwsContext *context, int src_colorspace, int dst_colorspace, int full_range ) { - int *coefficients; - const int *new_coefficients; - int full_range; - int brightness, contrast, saturation; + const int *src_coefficients = sws_getCoefficients( SWS_CS_DEFAULT ); + const int *dst_coefficients = sws_getCoefficients( SWS_CS_DEFAULT ); + int brightness = 0; + int contrast = 1 << 16; + int saturation = 1 << 16; - if ( sws_getColorspaceDetails( context, &coefficients, &full_range, &coefficients, &full_range, - &brightness, &contrast, &saturation ) != -1 ) + switch ( src_colorspace ) { - // Don't change these from defaults unless explicitly told to. - if ( use_full_range >= 0 ) - full_range = use_full_range; - switch ( colorspace ) - { - case 170: - case 470: - case 601: - case 624: - new_coefficients = sws_getCoefficients( SWS_CS_ITU601 ); - break; - case 240: - new_coefficients = sws_getCoefficients( SWS_CS_SMPTE240M ); - break; - case 709: - new_coefficients = sws_getCoefficients( SWS_CS_ITU709 ); - break; - default: - new_coefficients = coefficients; - break; - } - sws_setColorspaceDetails( context, new_coefficients, full_range, new_coefficients, full_range, - brightness, contrast, saturation ); + case 170: + case 470: + case 601: + case 624: + src_coefficients = sws_getCoefficients( SWS_CS_ITU601 ); + break; + case 240: + src_coefficients = sws_getCoefficients( SWS_CS_SMPTE240M ); + break; + case 709: + src_coefficients = sws_getCoefficients( SWS_CS_ITU709 ); + break; + default: + break; + } + switch ( dst_colorspace ) + { + case 170: + case 470: + case 601: + case 624: + src_coefficients = sws_getCoefficients( SWS_CS_ITU601 ); + break; + case 240: + src_coefficients = sws_getCoefficients( SWS_CS_SMPTE240M ); + break; + case 709: + src_coefficients = sws_getCoefficients( SWS_CS_ITU709 ); + break; + default: + break; } + return sws_setColorspaceDetails( context, src_coefficients, full_range, dst_coefficients, full_range, + brightness, contrast, saturation ); } -static void av_convert_image( uint8_t *out, uint8_t *in, int out_fmt, int in_fmt, - int width, int height, int colorspace, int use_full_range ) +// returns set_lumage_transfer result +static int av_convert_image( uint8_t *out, uint8_t *in, int out_fmt, int in_fmt, + int width, int height, int src_colorspace, int dst_colorspace, int use_full_range ) { AVPicture input; AVPicture output; int flags = SWS_BICUBIC | SWS_ACCURATE_RND; + int error = -1; if ( out_fmt == PIX_FMT_YUYV422 ) flags |= SWS_FULL_CHR_H_INP; @@ -121,6 +134,8 @@ static void av_convert_image( uint8_t *out, uint8_t *in, int out_fmt, int in_fmt #ifdef USE_SSE flags |= SWS_CPU_CAPS_MMX2; #endif + if ( out_fmt == PIX_FMT_YUV420P && use_full_range ) + out_fmt = PIX_FMT_YUVJ420P; avpicture_fill( &input, in, in_fmt, width, height ); avpicture_fill( &output, out, out_fmt, width, height ); @@ -128,11 +143,12 @@ static void av_convert_image( uint8_t *out, uint8_t *in, int out_fmt, int in_fmt width, height, out_fmt, flags, NULL, NULL, NULL); if ( context ) { - set_luma_transfer( context, colorspace, use_full_range ); + error = set_luma_transfer( context, src_colorspace, dst_colorspace, use_full_range ); sws_scale( context, (const uint8_t* const*) input.data, input.linesize, 0, height, output.data, output.linesize); sws_freeContext( context ); } + return error; } /** Do it :-). @@ -147,12 +163,14 @@ static int convert_image( mlt_frame frame, uint8_t **image, mlt_image_format *fo if ( *format != output_format ) { + mlt_profile profile = mlt_service_profile( + MLT_PRODUCER_SERVICE( mlt_frame_get_original_producer( frame ) ) ); int colorspace = mlt_properties_get_int( properties, "colorspace" ); int force_full_luma = -1; - mlt_log_debug( NULL, "[filter avcolor_space] %s -> %s @ %dx%d space %d\n", + mlt_log_debug( NULL, "[filter avcolor_space] %s -> %s @ %dx%d space %d->%d\n", mlt_image_format_name( *format ), mlt_image_format_name( output_format ), - width, height, colorspace ); + width, height, colorspace, profile->colorspace ); int in_fmt = convert_mlt_to_av_cs( *format ); int out_fmt = convert_mlt_to_av_cs( output_format ); @@ -196,10 +214,16 @@ static int convert_image( mlt_frame frame, uint8_t **image, mlt_image_format *fo // By removing the frame property we only permit the luma to skip scaling once. // Thereafter, we let swscale scale the luma range as it pleases since it seems // we do not have control over the RGB to YUV conversion. - force_full_luma = mlt_properties_get_int( properties, "force_full_luma" ); + force_full_luma = mlt_properties_get_int( properties, "force_full_luma" ); mlt_properties_set( properties, "force_full_luma", NULL ); } - av_convert_image( output, *image, out_fmt, in_fmt, width, height, colorspace, force_full_luma ); + if ( !av_convert_image( output, *image, out_fmt, in_fmt, width, height, + colorspace, profile->colorspace, force_full_luma ) ) + { + // The new colorspace is only valid if destination is YUV. + if ( output_format == mlt_image_yuv422 || output_format == mlt_image_yuv420p ) + mlt_properties_set_int( properties, "colorspace", profile->colorspace ); + } *image = output; *format = output_format; mlt_frame_set_image( frame, output, size, mlt_pool_release ); @@ -238,8 +262,7 @@ static int convert_image( mlt_frame frame, uint8_t **image, mlt_image_format *fo return error; } -/* TODO: The below is not working because swscale does not have - * adjustable coefficients yet for RGB->YUV */ +/* TODO: Enable this to force colorspace conversion. Cost is heavy due to RGB conversions. */ #if 0 static int get_image( mlt_frame frame, uint8_t **image, mlt_image_format *format, int *width, int *height, int writable ) { diff --git a/src/modules/avformat/producer_avformat.c b/src/modules/avformat/producer_avformat.c index f2530308..4fc10523 100644 --- a/src/modules/avformat/producer_avformat.c +++ b/src/modules/avformat/producer_avformat.c @@ -1026,40 +1026,50 @@ static void get_audio_streams_info( producer_avformat self ) self->audio_streams, self->audio_max_stream, self->total_channels, self->max_channel ); } -static void set_luma_transfer( struct SwsContext *context, int yuv_colorspace, int use_full_range ) +static int set_luma_transfer( struct SwsContext *context, int src_colorspace, int dst_colorspace, int full_range ) { - int *coefficients; - const int *new_coefficients; - int full_range; - int brightness, contrast, saturation; - - if ( sws_getColorspaceDetails( context, &coefficients, &full_range, &coefficients, &full_range, - &brightness, &contrast, &saturation ) != -1 ) - { - // Don't change these from defaults unless explicitly told to. - if ( use_full_range >= 0 ) - full_range = use_full_range; - switch ( yuv_colorspace ) - { - case 170: - case 470: - case 601: - case 624: - new_coefficients = sws_getCoefficients( SWS_CS_ITU601 ); - break; - case 240: - new_coefficients = sws_getCoefficients( SWS_CS_SMPTE240M ); - break; - case 709: - new_coefficients = sws_getCoefficients( SWS_CS_ITU709 ); - break; - default: - new_coefficients = coefficients; - break; - } - sws_setColorspaceDetails( context, new_coefficients, full_range, new_coefficients, full_range, - brightness, contrast, saturation ); + const int *src_coefficients = sws_getCoefficients( SWS_CS_DEFAULT ); + const int *dst_coefficients = sws_getCoefficients( SWS_CS_DEFAULT ); + int brightness = 0; + int contrast = 1 << 16; + int saturation = 1 << 16; + + switch ( src_colorspace ) + { + case 170: + case 470: + case 601: + case 624: + src_coefficients = sws_getCoefficients( SWS_CS_ITU601 ); + break; + case 240: + src_coefficients = sws_getCoefficients( SWS_CS_SMPTE240M ); + break; + case 709: + src_coefficients = sws_getCoefficients( SWS_CS_ITU709 ); + break; + default: + break; + } + switch ( dst_colorspace ) + { + case 170: + case 470: + case 601: + case 624: + src_coefficients = sws_getCoefficients( SWS_CS_ITU601 ); + break; + case 240: + src_coefficients = sws_getCoefficients( SWS_CS_SMPTE240M ); + break; + case 709: + src_coefficients = sws_getCoefficients( SWS_CS_ITU709 ); + break; + default: + break; } + return sws_setColorspaceDetails( context, src_coefficients, full_range, dst_coefficients, full_range, + brightness, contrast, saturation ); } static mlt_image_format pick_pix_format( enum PixelFormat pix_fmt ) @@ -1117,10 +1127,13 @@ static mlt_audio_format pick_audio_format( int sample_fmt ) } } -static void convert_image( producer_avformat self, AVFrame *frame, uint8_t *buffer, int pix_fmt, +// returns resulting YUV colorspace +static int convert_image( producer_avformat self, AVFrame *frame, uint8_t *buffer, int pix_fmt, mlt_image_format *format, int width, int height, uint8_t **alpha ) { int flags = SWS_BICUBIC | SWS_ACCURATE_RND; + mlt_profile profile = mlt_service_profile( MLT_PRODUCER_SERVICE( self->parent ) ); + int result = self->yuv_colorspace; #ifdef USE_MMX flags |= SWS_CPU_CAPS_MMX; @@ -1129,6 +1142,10 @@ static void convert_image( producer_avformat self, AVFrame *frame, uint8_t *buff flags |= SWS_CPU_CAPS_MMX2; #endif + mlt_log_debug( MLT_PRODUCER_SERVICE(self->parent), "%s @ %dx%d space %d->%d\n", + mlt_image_format_name( *format ), + width, height, self->yuv_colorspace, profile->colorspace ); + // extract alpha from planar formats if ( ( pix_fmt == PIX_FMT_YUVA420P #if defined(FFUDIV) && LIBAVUTIL_VERSION_INT >= ((51<<16)+(35<<8)+101) @@ -1151,7 +1168,8 @@ static void convert_image( producer_avformat self, AVFrame *frame, uint8_t *buff if ( *format == mlt_image_yuv420p ) { struct SwsContext *context = sws_getContext( width, height, pix_fmt, - width, height, PIX_FMT_YUV420P, flags, NULL, NULL, NULL); + width, height, self->full_luma ? PIX_FMT_YUVJ420P : PIX_FMT_YUV420P, + flags, NULL, NULL, NULL); AVPicture output; output.data[0] = buffer; output.data[1] = buffer + width * height; @@ -1159,7 +1177,8 @@ static void convert_image( producer_avformat self, AVFrame *frame, uint8_t *buff output.linesize[0] = width; output.linesize[1] = width >> 1; output.linesize[2] = width >> 1; - set_luma_transfer( context, self->yuv_colorspace, -1 ); + if ( !set_luma_transfer( context, self->yuv_colorspace, profile->colorspace, self->full_luma ) ) + result = profile->colorspace; sws_scale( context, (const uint8_t* const*) frame->data, frame->linesize, 0, height, output.data, output.linesize); sws_freeContext( context ); @@ -1170,7 +1189,7 @@ static void convert_image( producer_avformat self, AVFrame *frame, uint8_t *buff width, height, PIX_FMT_RGB24, flags | SWS_FULL_CHR_H_INT, NULL, NULL, NULL); AVPicture output; avpicture_fill( &output, buffer, PIX_FMT_RGB24, width, height ); - set_luma_transfer( context, self->yuv_colorspace, self->full_luma ); + set_luma_transfer( context, self->yuv_colorspace, profile->colorspace, self->full_luma ); sws_scale( context, (const uint8_t* const*) frame->data, frame->linesize, 0, height, output.data, output.linesize); sws_freeContext( context ); @@ -1181,7 +1200,7 @@ static void convert_image( producer_avformat self, AVFrame *frame, uint8_t *buff width, height, PIX_FMT_RGBA, flags | SWS_FULL_CHR_H_INT, NULL, NULL, NULL); AVPicture output; avpicture_fill( &output, buffer, PIX_FMT_RGBA, width, height ); - set_luma_transfer( context, self->yuv_colorspace, self->full_luma ); + set_luma_transfer( context, self->yuv_colorspace, profile->colorspace, self->full_luma ); sws_scale( context, (const uint8_t* const*) frame->data, frame->linesize, 0, height, output.data, output.linesize); sws_freeContext( context ); @@ -1192,11 +1211,13 @@ static void convert_image( producer_avformat self, AVFrame *frame, uint8_t *buff width, height, PIX_FMT_YUYV422, flags | SWS_FULL_CHR_H_INP, NULL, NULL, NULL); AVPicture output; avpicture_fill( &output, buffer, PIX_FMT_YUYV422, width, height ); - set_luma_transfer( context, self->yuv_colorspace, -1 ); + if ( !set_luma_transfer( context, self->yuv_colorspace, profile->colorspace, self->full_luma ) ) + result = profile->colorspace; sws_scale( context, (const uint8_t* const*) frame->data, frame->linesize, 0, height, output.data, output.linesize); sws_freeContext( context ); } + return result; } /** Allocate the image buffer and set it on the frame. @@ -1369,13 +1390,15 @@ static int producer_get_image( mlt_frame frame, uint8_t **buffer, mlt_image_form picture.linesize[0] = codec_context->width; picture.linesize[1] = codec_context->width / 2; picture.linesize[2] = codec_context->width / 2; - convert_image( self, (AVFrame*) &picture, *buffer, + int yuv_colorspace = convert_image( self, (AVFrame*) &picture, *buffer, PIX_FMT_YUV420P, format, *width, *height, &alpha ); + mlt_properties_set_int( frame_properties, "colorspace", yuv_colorspace ); } else #endif - convert_image( self, self->video_frame, *buffer, codec_context->pix_fmt, + int yuv_colorspace = convert_image( self, self->video_frame, *buffer, codec_context->pix_fmt, format, *width, *height, &alpha ); + mlt_properties_set_int( frame_properties, "colorspace", yuv_colorspace ); got_picture = 1; } } @@ -1551,8 +1574,9 @@ static int producer_get_image( mlt_frame frame, uint8_t **buffer, mlt_image_form VdpStatus status = vdp_surface_get_bits( render->surface, dest_format, planes, pitches ); if ( status == VDP_STATUS_OK ) { - convert_image( self, self->video_frame, *buffer, PIX_FMT_YUV420P, + int yuv_colorspace = convert_image( self, self->video_frame, *buffer, PIX_FMT_YUV420P, format, *width, *height, &alpha ); + mlt_properties_set_int( frame_properties, "colorspace", yuv_colorspace ); } else { @@ -1568,8 +1592,9 @@ static int producer_get_image( mlt_frame frame, uint8_t **buffer, mlt_image_form } else #endif - convert_image( self, self->video_frame, *buffer, codec_context->pix_fmt, + int yuv_colorspace = convert_image( self, self->video_frame, *buffer, codec_context->pix_fmt, format, *width, *height, &alpha ); + mlt_properties_set_int( frame_properties, "colorspace", yuv_colorspace ); self->top_field_first |= self->video_frame->top_field_first; self->current_position = int_position; } @@ -1617,13 +1642,15 @@ static int producer_get_image( mlt_frame frame, uint8_t **buffer, mlt_image_form picture.linesize[0] = codec_context->width; picture.linesize[1] = codec_context->width / 2; picture.linesize[2] = codec_context->width / 2; - convert_image( self, (AVFrame*) &picture, *buffer, + int yuv_colorspace = convert_image( self, (AVFrame*) &picture, *buffer, PIX_FMT_YUV420P, format, *width, *height, &alpha ); + mlt_properties_set_int( frame_properties, "colorspace", yuv_colorspace ); } else #endif - convert_image( self, self->video_frame, *buffer, codec_context->pix_fmt, + int yuv_colorspace = convert_image( self, self->video_frame, *buffer, codec_context->pix_fmt, format, *width, *height, &alpha ); + mlt_properties_set_int( frame_properties, "colorspace", yuv_colorspace ); got_picture = 1; } } @@ -1864,7 +1891,7 @@ static int video_codec_init( producer_avformat self, int index, mlt_properties p break; } - self->full_luma = -1; + self->full_luma = 0; #if LIBAVCODEC_VERSION_INT >= ((52<<16)+(72<<8)+2) mlt_log_debug( MLT_PRODUCER_SERVICE(self->parent), "color_range %d\n", codec_context->color_range ); if ( codec_context->color_range == AVCOL_RANGE_JPEG ) -- 2.39.2