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.

diff --git a/src/modules/avformat/filter_avcolour_space.c b/src/modules/avformat/filter_avcolour_space.c
index 2e5fc6244d597ad620ab315f67537e494687a71b..627a3c82179335ff7b1e529caf0f03c052fbeeca 100644 (file)
--- a/src/modules/avformat/filter_avcolour_space.c
+++ b/src/modules/avformat/filter_avcolour_space.c
@@ -22,6 +22,7 @@
 #include <framework/mlt_frame.h>
 #include <framework/mlt_log.h>
 #include <framework/mlt_profile.h>
+#include <framework/mlt_producer.h>
 
 // ffmpeg Header files
 #include <libavformat/avformat.h>
@@ -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 f2530308e8cfc553bb3e2dd2766691a96203d553..4fc105230ef2969e00cf842880054757d245a57c 100644 (file)
--- 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 )