[FFmpeg-devel] Fixes #10509

[Poorva]

>

On Tue, Mar 26, 2024 at 2:36 AM Poorva <2003gaikarpoorva at gmail.com> wrote:
>>
>>
>
> On Mon, Mar 25, 2024 at 2:04 AM Marton Balint <cus at passwd.hu> wrote:
>>
>>
>>
>> On Sun, 24 Mar 2024, Poorva wrote:
>>
>> > From 0874a55ad33bb4df2789cbafc5247c0ae3d97e6a Mon Sep 17 00:00:00 2001
>> > From: PoorvaGaikar <pbgaikar_b21 at el.vjti.ac.in>
>> > Date: Sat, 9 Mar 2024 00:27:25 +0530
>> > Subject: [PATCH v2] avfilter/f_select.c: add support for iw and ih constants
>>
>> You should mention the fixed ticket number in the commit message.
>>
>> >
>> > ---
>> >  doc/filters.texi       |  6 ++++++
>> >  libavfilter/f_select.c | 14 ++++++++++++++
>> >  2 files changed, 20 insertions(+)
>> >
>> > diff --git a/doc/filters.texi b/doc/filters.texi
>> > index 913365671d..e73dc9c1bf 100644
>> > --- a/doc/filters.texi
>> > +++ b/doc/filters.texi
>> > @@ -30737,6 +30737,12 @@ missing.
>> >  That basically means that an input frame is selected if its pts is within the
>> >  interval set by the concat demuxer.
>> >
>> > + at item iw @emph{(video only)}
>> > +Represents the width of the input video frame
>> > +
>> > + at item ih @emph{(video only)}
>> > +Represents the height of the input video frame
>> > +
>> >  @end table
>> >
>> >  The default value of the select expression is "1".
>> > diff --git a/libavfilter/f_select.c b/libavfilter/f_select.c
>> > index 9b330a0673..ccfe915ee1 100644
>> > --- a/libavfilter/f_select.c
>> > +++ b/libavfilter/f_select.c
>> > @@ -90,6 +90,9 @@ static const char *const var_names[] = {
>> >
>> >      "concatdec_select",  ///< frame is within the interval set by the concat demuxer
>> >
>> > +    "ih",                ///< ih: Represents the height of the input video frame.
>> > +    "iw",                ///< iw: Represents the width of the input video frame.
>> > +
>> >      NULL
>> >  };
>> >
>> > @@ -144,6 +147,9 @@ enum var_name {
>> >
>> >      VAR_CONCATDEC_SELECT,
>> >
>> > +    VAR_IH,
>> > +    VAR_IW,
>> > +
>> >      VAR_VARS_NB
>> >  };
>> >
>> > @@ -264,6 +270,9 @@ static int config_input(AVFilterLink *inlink)
>> >      select->var_values[VAR_CONSUMED_SAMPLES_N] = NAN;
>> >      select->var_values[VAR_SAMPLES_N]          = NAN;
>> >
>> > +    select->var_values[VAR_IH] = NAN;
>> > +    select->var_values[VAR_IW] = NAN;
>> > +
>> >      select->var_values[VAR_SAMPLE_RATE] =
>> >          inlink->type == AVMEDIA_TYPE_AUDIO ? inlink->sample_rate : NAN;
>> >
>> > @@ -370,6 +379,10 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> >          }
>> >          break;
>> >      }
>> > +    if (inlink->type == AVMEDIA_TYPE_VIDEO){
>> > +        select->var_values[VAR_IH] = frame->height;
>> > +        select->var_values[VAR_IW] = frame->width;
>> > +    }
>>
>> There is a switch(inlink->type) just above this, so you should put these
>> there to the corresponding case AVMEDIA_TYPE_VIDEO, and not create a new
>> block.
>>
>> >
>> >      select->select = res = av_expr_eval(select->expr, select->var_values, NULL);
>> >      av_log(inlink->dst, AV_LOG_DEBUG,
>> > @@ -546,3 +559,4 @@ const AVFilter ff_vf_select = {
>> >      .flags         = AVFILTER_FLAG_DYNAMIC_OUTPUTS | AVFILTER_FLAG_METADATA_ONLY,
>> >  };
>> >  #endif /* CONFIG_SELECT_FILTER */
>> > +
>>
>> This is still an uneeded newline.
>
>
> Thank you for your feedback on the Git patch I submitted for review.
> I have rectified the problem by adding the necessary changes .
> The updated patch file is attached for your review.
>>
>>
 I wanted to follow up on the patch titled
"v3-0001-avfilter-f_select.c - Add Support for IW and IH" that I
submitted earlier and provide an update based on the feedback
received.

In response to your suggestion about the switch block, I have
integrated the changes into the existing switch block for
AVMEDIA_TYPE_VIDEO. Additionally, I have removed an unnecessary new
line that was added at the end of the file.

Despite these modifications, I have not received any further feedback
or comments on the patch. Therefore, I kindly request the community to
review the updated patch attached to this email.
>>
>> Regards,
>> Marton
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".