[FFmpeg-devel] [PATCHv2] vf_scale: Use correct chroma position in YUV420P interlaced conversions

[Michael Niedermayer]

On Wed, Dec 24, 2014 at 03:10:56PM +0000, Kieran Kunhya wrote:
> Fixed wrong chroma line use
> ---
>  libavfilter/vf_scale.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index 64b88c2..9189103 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -373,6 +373,13 @@ static int config_props(AVFilterLink *outlink)
>              av_opt_set_int(*s, "dst_format", outfmt, 0);
>              av_opt_set_int(*s, "sws_flags", scale->flags, 0);
>  
> +            /* Override interlaced YUV420P settings to have the correct (MPEG-2) chroma positions
> +             * MPEG-2 chroma positions are used by convention
> +             * Set the context up for tff */
> +            if (i && scale->interlaced && inlink->format == AV_PIX_FMT_YUV420P){
> +                scale->in_v_chr_pos = (i == 1) ? 64 : -64;
> +            }

this should check if in_v_chr_pos is the default, and maybe either
not change it if its not or print a warning to inform the user that
the value she specified is not being used
maybe av_opt_is_set_to_default_by_name() could be used
the default also could be changed to detect if the user explicitly
specified the "default" value but maybe thats overkill feel free to
ignore


> +
>              av_opt_set_int(*s, "src_h_chr_pos", scale->in_h_chr_pos, 0);
>              av_opt_set_int(*s, "src_v_chr_pos", scale->in_v_chr_pos, 0);
>              av_opt_set_int(*s, "dst_h_chr_pos", scale->out_h_chr_pos, 0);
> @@ -520,8 +527,8 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>                INT_MAX);
>  
>      if(scale->interlaced>0 || (scale->interlaced<0 && in->interlaced_frame)){
> -        scale_slice(link, out, in, scale->isws[0], 0, (link->h+1)/2, 2, 0);
> -        scale_slice(link, out, in, scale->isws[1], 0,  link->h   /2, 2, 1);
> +        scale_slice(link, out, in, scale->isws[!in->top_field_first], 0, (link->h+1)/2, 2, 0);
> +        scale_slice(link, out, in, scale->isws[in->top_field_first],  0,  link->h   /2, 2, 1);
>      }else{

are you sure this is correct ?
maybe i misunderstand but it looks a bit odd
i think the upper field(s) should be treated the same no matter if
its temporally first or second

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141224/18d5b20f/attachment.asc>