[FFmpeg-devel] [PATCH 1/2] fftools/ffplay: add missing YUV metadata to buffersrc

[FFmpeg-devel] [PATCH 1/2] fftools/ffplay: add missing YUV metadata to buffersrc

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Fixes error spam from the `ffplay` tool since commit 2d555dc82d, caused
by an oversight on my part - I didn't notice during development that
`ffplay` goes through its own filtering code path separate from
fftools/ffmpeg_filter.c
---
 fftools/ffplay.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index 17861e60be..0771326d76 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -1881,10 +1881,12 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
     graph->scale_sws_opts = av_strdup(sws_flags_str);
 
     snprintf(buffersrc_args, sizeof(buffersrc_args),
-             "video_size=%dx%d:pix_fmt=%d:time_base=%d/%d:pixel_aspect=%d/%d",
+             "video_size=%dx%d:pix_fmt=%d:time_base=%d/%d:pixel_aspect=%d/%d:"
+             "colorspace=%d:range=%d",
              frame->width, frame->height, frame->format,
              is->video_st->time_base.num, is->video_st->time_base.den,
-             codecpar->sample_aspect_ratio.num, FFMAX(codecpar->sample_aspect_ratio.den, 1));
+             codecpar->sample_aspect_ratio.num, FFMAX(codecpar->sample_aspect_ratio.den, 1),
+             frame->colorspace, frame->color_range);
     if (fr.num && fr.den)
         av_strlcatf(buffersrc_args, sizeof(buffersrc_args), ":frame_rate=%d/%d", fr.num, fr.den);
 
-- 
2.43.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/2] fftools/ffmpeg_filter: simplify buffersrc arg printing

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

There's no need to go through full string expansion here.
---
 fftools/ffmpeg_filter.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 82ee4fae7d..f4324579c5 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -1497,11 +1497,10 @@ static int configure_input_video_filter(FilterGraph *fg, AVFilterGraph *graph,
     av_bprint_init(&args, 0, AV_BPRINT_SIZE_AUTOMATIC);
     av_bprintf(&args,
              "video_size=%dx%d:pix_fmt=%d:time_base=%d/%d:"
-             "pixel_aspect=%d/%d:colorspace=%s:range=%s",
+             "pixel_aspect=%d/%d:colorspace=%d:range=%d",
              ifp->width, ifp->height, ifp->format,
              ifp->time_base.num, ifp->time_base.den, sar.num, sar.den,
-             av_color_space_name(ifp->color_space),
-             av_color_range_name(ifp->color_range));
+             ifp->color_space, ifp->color_range);
     if (fr.num && fr.den)
         av_bprintf(&args, ":frame_rate=%d/%d", fr.num, fr.den);
     snprintf(name, sizeof(name), "graph %d input from stream %d:%d", fg->index,
-- 
2.43.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] fftools/ffplay: add missing YUV metadata to buffersrc

[Anton Khirnov]

Quoting Niklas Haas (2024-01-10 10:05:46)
> From: Niklas Haas <git@haasn.dev>
> 
> Fixes error spam from the `ffplay` tool since commit 2d555dc82d, caused
> by an oversight on my part - I didn't notice during development that
> `ffplay` goes through its own filtering code path separate from
> fftools/ffmpeg_filter.c

Wouldn't the same issue affect any other caller?

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] fftools/ffplay: add missing YUV metadata to buffersrc

[Zhao Zhili]

> On Jan 26, 2024, at 17:36, Anton Khirnov <anton@khirnov.net> wrote:
> 
> Quoting Niklas Haas (2024-01-10 10:05:46)
>> From: Niklas Haas <git@haasn.dev>
>> 
>> Fixes error spam from the `ffplay` tool since commit 2d555dc82d, caused
>> by an oversight on my part - I didn't notice during development that
>> `ffplay` goes through its own filtering code path separate from
>> fftools/ffmpeg_filter.c
> 
> Wouldn't the same issue affect any other caller?

Firstly, can avfilter support color range/space change from unspecified to a specified value
on the first frame? I think it should silent the log most of the cases.

Secondly, log once for non-serious case.

> 
> -- 
> Anton Khirnov
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] fftools/ffplay: add missing YUV metadata to buffersrc

[Niklas Haas]

On Fri, 26 Jan 2024 18:17:30 +0800 Zhao Zhili <quinkblack@foxmail.com> wrote:
> 
> 
> > On Jan 26, 2024, at 17:36, Anton Khirnov <anton@khirnov.net> wrote:
> > 
> > Quoting Niklas Haas (2024-01-10 10:05:46)
> >> From: Niklas Haas <git@haasn.dev>
> >> 
> >> Fixes error spam from the `ffplay` tool since commit 2d555dc82d, caused
> >> by an oversight on my part - I didn't notice during development that
> >> `ffplay` goes through its own filtering code path separate from
> >> fftools/ffmpeg_filter.c
> > 
> > Wouldn't the same issue affect any other caller?
> 
> Firstly, can avfilter support color range/space change from unspecified to a specified value
> on the first frame? I think it should silent the log most of the cases.
> 
> Secondly, log once for non-serious case.

To clarify, commit 94422871fce3b90bebc95f5cae939fbbc4e33224 makes this
message drop from WARNING to DEBUG verbosity for subsequent print-outs.

So the error spam is no longer a major consideration, however it remains
that ffplay needs to set the correct metadata for YUV colorspace
negotiation to work inside the ffplay filter graph. (See the issue
I linked in my other mail)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] fftools/ffplay: add missing YUV metadata to buffersrc

[Niklas Haas]

On Wed, 10 Jan 2024 10:05:46 +0100 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> Fixes error spam from the `ffplay` tool since commit 2d555dc82d, caused
> by an oversight on my part - I didn't notice during development that
> `ffplay` goes through its own filtering code path separate from
> fftools/ffmpeg_filter.c
> ---
>  fftools/ffplay.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index 17861e60be..0771326d76 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -1881,10 +1881,12 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>      graph->scale_sws_opts = av_strdup(sws_flags_str);
>  
>      snprintf(buffersrc_args, sizeof(buffersrc_args),
> -             "video_size=%dx%d:pix_fmt=%d:time_base=%d/%d:pixel_aspect=%d/%d",
> +             "video_size=%dx%d:pix_fmt=%d:time_base=%d/%d:pixel_aspect=%d/%d:"
> +             "colorspace=%d:range=%d",
>               frame->width, frame->height, frame->format,
>               is->video_st->time_base.num, is->video_st->time_base.den,
> -             codecpar->sample_aspect_ratio.num, FFMAX(codecpar->sample_aspect_ratio.den, 1));
> +             codecpar->sample_aspect_ratio.num, FFMAX(codecpar->sample_aspect_ratio.den, 1),
> +             frame->colorspace, frame->color_range);
>      if (fr.num && fr.den)
>          av_strlcatf(buffersrc_args, sizeof(buffersrc_args), ":frame_rate=%d/%d", fr.num, fr.den);
>  
> -- 
> 2.43.0
> 

Will merge tomorrow if there is no further objection, especially as it
now affects https://trac.ffmpeg.org/ticket/10839
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".