Re: [FFmpeg-devel] [WIP][RFC][PATCH] avfilter: negotiate color_range between filters

2017-12-11 Thread Paul B Mahol 
On 12/11/17, Nicolas George  wrote:
> I do not have time to look at the code in details right now, and I do
> not know when I will have time, but here are a few remarks.
>
> Paul B Mahol (2017-12-11):
>> Signed-off-by: Paul B Mahol 
>> ---
>>
>> This is with color range negotiation.
>> To be applied on top of previous patch set.
>> It fixes issue Michael found.
>
>> This needs more work as every filter needs its query_formats() changed.
>
> This is annoying. Could you not get the framework set a reasonable value
> (all supported) when the list is not set?

Done locally.

>
>>
>> ---
>>  fftools/ffmpeg_filter.c | 13 +++---
>>  libavfilter/avfilter.c  |  9 ---
>>  libavfilter/avfilter.h  |  4 ++-
>>  libavfilter/avfiltergraph.c | 34 
>>  libavfilter/buffersink.c| 16 
>>  libavfilter/buffersink.h|  1 +
>>  libavfilter/buffersrc.c |  4 +++
>>  libavfilter/formats.c   | 63
>> +
>>  libavfilter/formats.h   | 31 ++
>>  libavfilter/internal.h  | 11 
>>  libavfilter/vf_format.c | 46 -
>>  libavfilter/vf_noise.c  |  6 -
>>  libavfilter/vf_scale.c  | 19 +++---
>>  libavfilter/vsrc_testsrc.c  | 15 +--
>>  14 files changed, 251 insertions(+), 21 deletions(-)
>>
>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
>> index 3aad564b81..a8fbcdecfb 100644
>> --- a/fftools/ffmpeg_filter.c
>> +++ b/fftools/ffmpeg_filter.c
>> @@ -144,6 +144,8 @@ static char *choose_pix_fmts(OutputFilter *ofilter)
>>  const char *name = av_get_pix_fmt_name(*p);
>>  avio_printf(s, "%s|", name);
>>  }
>> +if (av_color_range_name(ost->enc->color_range))
>> +avio_printf(s, ":color_ranges=%s|",
>> av_color_range_name(ost->enc->color_range));
>>  len = avio_close_dyn_buf(s, );
>>  ret[len - 1] = 0;
>>  return ret;
>> @@ -447,7 +449,6 @@ static int configure_output_video_filter(FilterGraph
>> *fg, OutputFilter *ofilter,
>>  OutputStream *ost = ofilter->ost;
>>  OutputFile*of = output_files[ost->file_index];
>>  AVFilterContext *last_filter = out->filter_ctx;
>> -AVDictionaryEntry *cre = NULL;
>>  int pad_idx = out->pad_idx;
>>  int ret;
>>  char name[255];
>> @@ -460,9 +461,7 @@ static int configure_output_video_filter(FilterGraph
>> *fg, OutputFilter *ofilter,
>>  if (ret < 0)
>>  return ret;
>>
>> -cre = av_dict_get(ost->encoder_opts, "color_range", NULL, 0);
>> -
>> -if (ofilter->width || ofilter->height || (cre && cre->value) ||
>> ost->enc->color_range) {
>> +if (ofilter->width || ofilter->height) {
>>  char args[255];
>>  AVFilterContext *filter;
>>  AVDictionaryEntry *e = NULL;
>> @@ -475,12 +474,6 @@ static int configure_output_video_filter(FilterGraph
>> *fg, OutputFilter *ofilter,
>>  av_strlcatf(args, sizeof(args), ":%s=%s", e->key, e->value);
>>  }
>>
>> -if (cre && cre->value) {
>> -av_strlcatf(args, sizeof(args), ":out_range=%s",
>> cre->value);
>> -} else if (ost->enc->color_range) {
>> -av_strlcatf(args, sizeof(args), ":out_range=%s",
>> av_color_range_name(ost->enc->color_range));
>> -}
>> -
>>  snprintf(name, sizeof(name), "scaler_out_%d_%d",
>>   ost->file_index, ost->index);
>>  if ((ret = avfilter_graph_create_filter(,
>> avfilter_get_by_name("scale"),
>> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
>> index 4a579bb49d..c3a19d100c 100644
>> --- a/libavfilter/avfilter.c
>> +++ b/libavfilter/avfilter.c
>> @@ -262,6 +262,9 @@ int avfilter_insert_filter(AVFilterLink *link,
>> AVFilterContext *filt,
>>  if (link->out_formats)
>>  ff_formats_changeref(>out_formats,
>>
>> >outputs[filt_dstpad_idx]->out_formats);
>> +if (link->out_color_ranges)
>> +ff_formats_changeref(>out_color_ranges,
>> +
>> >outputs[filt_dstpad_idx]->out_color_ranges);
>>  if (link->out_samplerates)
>>  ff_formats_changeref(>out_samplerates,
>>
>> >outputs[filt_dstpad_idx]->out_samplerates);
>> @@ -785,6 +788,8 @@ static void free_link(AVFilterLink *link)
>>
>>  ff_formats_unref(>in_formats);
>>  ff_formats_unref(>out_formats);
>> +ff_formats_unref(>in_color_ranges);
>> +ff_formats_unref(>out_color_ranges);
>>  ff_formats_unref(>in_samplerates);
>>  ff_formats_unref(>out_samplerates);
>>  ff_channel_layouts_unref(>in_channel_layouts);
>> @@ -970,9 +975,7 @@ int avfilter_init_str(AVFilterContext *filter, const
>> char *args)
>>  }
>>
>>  #if FF_API_OLD_FILTER_OPTS_ERROR
>> -if (   !strcmp(filter->filter->name, "format") ||
>> -   !strcmp(filter->filter->name, "noformat")   ||
>> -   !strcmp(filter->filter->name, "frei0r")

Re: [FFmpeg-devel] [WIP][RFC][PATCH] avfilter: negotiate color_range between filters

2017-12-11 Thread Nicolas George 
I do not have time to look at the code in details right now, and I do
not know when I will have time, but here are a few remarks.

Paul B Mahol (2017-12-11):
> Signed-off-by: Paul B Mahol 
> ---
> 
> This is with color range negotiation.
> To be applied on top of previous patch set.
> It fixes issue Michael found.

> This needs more work as every filter needs its query_formats() changed.

This is annoying. Could you not get the framework set a reasonable value
(all supported) when the list is not set?

> 
> ---
>  fftools/ffmpeg_filter.c | 13 +++---
>  libavfilter/avfilter.c  |  9 ---
>  libavfilter/avfilter.h  |  4 ++-
>  libavfilter/avfiltergraph.c | 34 
>  libavfilter/buffersink.c| 16 
>  libavfilter/buffersink.h|  1 +
>  libavfilter/buffersrc.c |  4 +++
>  libavfilter/formats.c   | 63 
> +
>  libavfilter/formats.h   | 31 ++
>  libavfilter/internal.h  | 11 
>  libavfilter/vf_format.c | 46 -
>  libavfilter/vf_noise.c  |  6 -
>  libavfilter/vf_scale.c  | 19 +++---
>  libavfilter/vsrc_testsrc.c  | 15 +--
>  14 files changed, 251 insertions(+), 21 deletions(-)
> 
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index 3aad564b81..a8fbcdecfb 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -144,6 +144,8 @@ static char *choose_pix_fmts(OutputFilter *ofilter)
>  const char *name = av_get_pix_fmt_name(*p);
>  avio_printf(s, "%s|", name);
>  }
> +if (av_color_range_name(ost->enc->color_range))
> +avio_printf(s, ":color_ranges=%s|", 
> av_color_range_name(ost->enc->color_range));
>  len = avio_close_dyn_buf(s, );
>  ret[len - 1] = 0;
>  return ret;
> @@ -447,7 +449,6 @@ static int configure_output_video_filter(FilterGraph *fg, 
> OutputFilter *ofilter,
>  OutputStream *ost = ofilter->ost;
>  OutputFile*of = output_files[ost->file_index];
>  AVFilterContext *last_filter = out->filter_ctx;
> -AVDictionaryEntry *cre = NULL;
>  int pad_idx = out->pad_idx;
>  int ret;
>  char name[255];
> @@ -460,9 +461,7 @@ static int configure_output_video_filter(FilterGraph *fg, 
> OutputFilter *ofilter,
>  if (ret < 0)
>  return ret;
>  
> -cre = av_dict_get(ost->encoder_opts, "color_range", NULL, 0);
> -
> -if (ofilter->width || ofilter->height || (cre && cre->value) || 
> ost->enc->color_range) {
> +if (ofilter->width || ofilter->height) {
>  char args[255];
>  AVFilterContext *filter;
>  AVDictionaryEntry *e = NULL;
> @@ -475,12 +474,6 @@ static int configure_output_video_filter(FilterGraph 
> *fg, OutputFilter *ofilter,
>  av_strlcatf(args, sizeof(args), ":%s=%s", e->key, e->value);
>  }
>  
> -if (cre && cre->value) {
> -av_strlcatf(args, sizeof(args), ":out_range=%s", cre->value);
> -} else if (ost->enc->color_range) {
> -av_strlcatf(args, sizeof(args), ":out_range=%s", 
> av_color_range_name(ost->enc->color_range));
> -}
> -
>  snprintf(name, sizeof(name), "scaler_out_%d_%d",
>   ost->file_index, ost->index);
>  if ((ret = avfilter_graph_create_filter(, 
> avfilter_get_by_name("scale"),
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 4a579bb49d..c3a19d100c 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -262,6 +262,9 @@ int avfilter_insert_filter(AVFilterLink *link, 
> AVFilterContext *filt,
>  if (link->out_formats)
>  ff_formats_changeref(>out_formats,
>   >outputs[filt_dstpad_idx]->out_formats);
> +if (link->out_color_ranges)
> +ff_formats_changeref(>out_color_ranges,
> + 
> >outputs[filt_dstpad_idx]->out_color_ranges);
>  if (link->out_samplerates)
>  ff_formats_changeref(>out_samplerates,
>   
> >outputs[filt_dstpad_idx]->out_samplerates);
> @@ -785,6 +788,8 @@ static void free_link(AVFilterLink *link)
>  
>  ff_formats_unref(>in_formats);
>  ff_formats_unref(>out_formats);
> +ff_formats_unref(>in_color_ranges);
> +ff_formats_unref(>out_color_ranges);
>  ff_formats_unref(>in_samplerates);
>  ff_formats_unref(>out_samplerates);
>  ff_channel_layouts_unref(>in_channel_layouts);
> @@ -970,9 +975,7 @@ int avfilter_init_str(AVFilterContext *filter, const char 
> *args)
>  }
>  
>  #if FF_API_OLD_FILTER_OPTS_ERROR
> -if (   !strcmp(filter->filter->name, "format") ||
> -   !strcmp(filter->filter->name, "noformat")   ||
> -   !strcmp(filter->filter->name, "frei0r") ||
> +if (   !strcmp(filter->filter->name, "frei0r") ||
>

[FFmpeg-devel] [WIP][RFC][PATCH] avfilter: negotiate color_range between filters

2017-12-11 Thread Paul B Mahol 
Signed-off-by: Paul B Mahol 
---

This is with color range negotiation.
To be applied on top of previous patch set.
It fixes issue Michael found.
This needs more work as every filter needs its query_formats() changed.

---
 fftools/ffmpeg_filter.c | 13 +++---
 libavfilter/avfilter.c  |  9 ---
 libavfilter/avfilter.h  |  4 ++-
 libavfilter/avfiltergraph.c | 34 
 libavfilter/buffersink.c| 16 
 libavfilter/buffersink.h|  1 +
 libavfilter/buffersrc.c |  4 +++
 libavfilter/formats.c   | 63 +
 libavfilter/formats.h   | 31 ++
 libavfilter/internal.h  | 11 
 libavfilter/vf_format.c | 46 -
 libavfilter/vf_noise.c  |  6 -
 libavfilter/vf_scale.c  | 19 +++---
 libavfilter/vsrc_testsrc.c  | 15 +--
 14 files changed, 251 insertions(+), 21 deletions(-)

diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 3aad564b81..a8fbcdecfb 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -144,6 +144,8 @@ static char *choose_pix_fmts(OutputFilter *ofilter)
 const char *name = av_get_pix_fmt_name(*p);
 avio_printf(s, "%s|", name);
 }
+if (av_color_range_name(ost->enc->color_range))
+avio_printf(s, ":color_ranges=%s|", 
av_color_range_name(ost->enc->color_range));
 len = avio_close_dyn_buf(s, );
 ret[len - 1] = 0;
 return ret;
@@ -447,7 +449,6 @@ static int configure_output_video_filter(FilterGraph *fg, 
OutputFilter *ofilter,
 OutputStream *ost = ofilter->ost;
 OutputFile*of = output_files[ost->file_index];
 AVFilterContext *last_filter = out->filter_ctx;
-AVDictionaryEntry *cre = NULL;
 int pad_idx = out->pad_idx;
 int ret;
 char name[255];
@@ -460,9 +461,7 @@ static int configure_output_video_filter(FilterGraph *fg, 
OutputFilter *ofilter,
 if (ret < 0)
 return ret;
 
-cre = av_dict_get(ost->encoder_opts, "color_range", NULL, 0);
-
-if (ofilter->width || ofilter->height || (cre && cre->value) || 
ost->enc->color_range) {
+if (ofilter->width || ofilter->height) {
 char args[255];
 AVFilterContext *filter;
 AVDictionaryEntry *e = NULL;
@@ -475,12 +474,6 @@ static int configure_output_video_filter(FilterGraph *fg, 
OutputFilter *ofilter,
 av_strlcatf(args, sizeof(args), ":%s=%s", e->key, e->value);
 }
 
-if (cre && cre->value) {
-av_strlcatf(args, sizeof(args), ":out_range=%s", cre->value);
-} else if (ost->enc->color_range) {
-av_strlcatf(args, sizeof(args), ":out_range=%s", 
av_color_range_name(ost->enc->color_range));
-}
-
 snprintf(name, sizeof(name), "scaler_out_%d_%d",
  ost->file_index, ost->index);
 if ((ret = avfilter_graph_create_filter(, 
avfilter_get_by_name("scale"),
diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index 4a579bb49d..c3a19d100c 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -262,6 +262,9 @@ int avfilter_insert_filter(AVFilterLink *link, 
AVFilterContext *filt,
 if (link->out_formats)
 ff_formats_changeref(>out_formats,
  >outputs[filt_dstpad_idx]->out_formats);
+if (link->out_color_ranges)
+ff_formats_changeref(>out_color_ranges,
+ 
>outputs[filt_dstpad_idx]->out_color_ranges);
 if (link->out_samplerates)
 ff_formats_changeref(>out_samplerates,
  >outputs[filt_dstpad_idx]->out_samplerates);
@@ -785,6 +788,8 @@ static void free_link(AVFilterLink *link)
 
 ff_formats_unref(>in_formats);
 ff_formats_unref(>out_formats);
+ff_formats_unref(>in_color_ranges);
+ff_formats_unref(>out_color_ranges);
 ff_formats_unref(>in_samplerates);
 ff_formats_unref(>out_samplerates);
 ff_channel_layouts_unref(>in_channel_layouts);
@@ -970,9 +975,7 @@ int avfilter_init_str(AVFilterContext *filter, const char 
*args)
 }
 
 #if FF_API_OLD_FILTER_OPTS_ERROR
-if (   !strcmp(filter->filter->name, "format") ||
-   !strcmp(filter->filter->name, "noformat")   ||
-   !strcmp(filter->filter->name, "frei0r") ||
+if (   !strcmp(filter->filter->name, "frei0r") ||
!strcmp(filter->filter->name, "frei0r_src") ||
!strcmp(filter->filter->name, "ocv")||
!strcmp(filter->filter->name, "pan")||
diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
index 40ad28ffd8..138bdeb5c9 100644
--- a/libavfilter/avfilter.h
+++ b/libavfilter/avfilter.h
@@ -456,7 +456,7 @@ struct AVFilterLink {
  *
  */
 /**
- * Lists of formats and