diff mbox series

[FFmpeg-devel] avfilter: propagate colorspace and color_range from buffer filter and between AVFilterLink.

Message ID 20240322210239.68833-1-damiog@gmail.com
State New
Headers show
Series [FFmpeg-devel] avfilter: propagate colorspace and color_range from buffer filter and between AVFilterLink. | expand

Checks

Context Check Description
yinshiyou/commit_msg_loongarch64 warning Please wrap lines in the body of the commit message between 60 and 72 characters.
andriy/commit_msg_x86 warning Please wrap lines in the body of the commit message between 60 and 72 characters.
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed

Commit Message

Damiano Galassi March 22, 2024, 9:02 p.m. UTC
There two new fields were never sent down the filter chain, and no filter after the first had colorspace and color_range set, causing breakage in zscale and possible other filters.
---
 libavfilter/avfilter.c  | 4 ++++
 libavfilter/buffersrc.c | 2 ++
 2 files changed, 6 insertions(+)

Comments

Niklas Haas March 23, 2024, 12:11 p.m. UTC | #1
On Fri, 22 Mar 2024 22:02:39 +0100 Damiano Galassi <damiog@gmail.com> wrote:
> There two new fields were never sent down the filter chain, and no filter after the first had colorspace and color_range set, causing breakage in zscale and possible other filters.
> ---
>  libavfilter/avfilter.c  | 4 ++++
>  libavfilter/buffersrc.c | 2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 831871de90..153fb700d3 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -391,6 +391,10 @@ int ff_filter_config_links(AVFilterContext *filter)
>                          link->w = inlink->w;
>                      if (!link->h)
>                          link->h = inlink->h;
> +                    if (link->colorspace == AVCOL_SPC_UNSPECIFIED)
> +                        link->colorspace = inlink->color_range;
> +                    if (link->color_range == AVCOL_RANGE_UNSPECIFIED)
> +                        link->color_range = inlink->color_range;
>                  } else if (!link->w || !link->h) {
>                      av_log(link->src, AV_LOG_ERROR,
>                             "Video source filters must set their output link's "
> diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
> index ddcd403785..2760097edf 100644
> --- a/libavfilter/buffersrc.c
> +++ b/libavfilter/buffersrc.c
> @@ -499,6 +499,8 @@ static int config_props(AVFilterLink *link)
>          link->w = c->w;
>          link->h = c->h;
>          link->sample_aspect_ratio = c->pixel_aspect;
> +        link->colorspace = c->color_space;
> +        link->color_range = c->color_range;
>  
>          if (c->hw_frames_ctx) {
>              link->hw_frames_ctx = av_buffer_ref(c->hw_frames_ctx);

LGTM, good catch!
Damiano Galassi March 23, 2024, 12:29 p.m. UTC | #2
On Sat, Mar 23, 2024 at 1:11 PM Niklas Haas <ffmpeg@haasn.xyz> wrote:

> On Fri, 22 Mar 2024 22:02:39 +0100 Damiano Galassi <damiog@gmail.com>
> wrote:
> > There two new fields were never sent down the filter chain, and no
> filter after the first had colorspace and color_range set, causing breakage
> in zscale and possible other filters.
> > ---
> >  libavfilter/avfilter.c  | 4 ++++
> >  libavfilter/buffersrc.c | 2 ++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> > index 831871de90..153fb700d3 100644
> > --- a/libavfilter/avfilter.c
> > +++ b/libavfilter/avfilter.c
> > @@ -391,6 +391,10 @@ int ff_filter_config_links(AVFilterContext *filter)
> >                          link->w = inlink->w;
> >                      if (!link->h)
> >                          link->h = inlink->h;
> > +                    if (link->colorspace == AVCOL_SPC_UNSPECIFIED)
> > +                        link->colorspace = inlink->color_range;
> > +                    if (link->color_range == AVCOL_RANGE_UNSPECIFIED)
> > +                        link->color_range = inlink->color_range;
> >                  } else if (!link->w || !link->h) {
> >                      av_log(link->src, AV_LOG_ERROR,
> >                             "Video source filters must set their output
> link's "
> > diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
> > index ddcd403785..2760097edf 100644
> > --- a/libavfilter/buffersrc.c
> > +++ b/libavfilter/buffersrc.c
> > @@ -499,6 +499,8 @@ static int config_props(AVFilterLink *link)
> >          link->w = c->w;
> >          link->h = c->h;
> >          link->sample_aspect_ratio = c->pixel_aspect;
> > +        link->colorspace = c->color_space;
> > +        link->color_range = c->color_range;
> >
> >          if (c->hw_frames_ctx) {
> >              link->hw_frames_ctx = av_buffer_ref(c->hw_frames_ctx);
>
> LGTM, good catch!
>

Unfortunately it breaks some tests:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240322210239.68833-1-damiog@gmail.com/
I didn't check yet what's going on there.
diff mbox series

Patch

diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index 831871de90..153fb700d3 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -391,6 +391,10 @@  int ff_filter_config_links(AVFilterContext *filter)
                         link->w = inlink->w;
                     if (!link->h)
                         link->h = inlink->h;
+                    if (link->colorspace == AVCOL_SPC_UNSPECIFIED)
+                        link->colorspace = inlink->color_range;
+                    if (link->color_range == AVCOL_RANGE_UNSPECIFIED)
+                        link->color_range = inlink->color_range;
                 } else if (!link->w || !link->h) {
                     av_log(link->src, AV_LOG_ERROR,
                            "Video source filters must set their output link's "
diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
index ddcd403785..2760097edf 100644
--- a/libavfilter/buffersrc.c
+++ b/libavfilter/buffersrc.c
@@ -499,6 +499,8 @@  static int config_props(AVFilterLink *link)
         link->w = c->w;
         link->h = c->h;
         link->sample_aspect_ratio = c->pixel_aspect;
+        link->colorspace = c->color_space;
+        link->color_range = c->color_range;
 
         if (c->hw_frames_ctx) {
             link->hw_frames_ctx = av_buffer_ref(c->hw_frames_ctx);