[FFmpeg-devel] [PATCH 3/4] fftools/opt_common: stop accessing a private field

[James Almer]

On 10/15/2024 6:57 AM, Anton Khirnov wrote:
> Quoting James Almer (2024-10-14 22:48:12)
>> On 10/14/2024 1:27 PM, Alexander Strasser via ffmpeg-devel wrote:
>>> On 2024-10-14 17:52 +0200, Michael Niedermayer wrote:
>>>> On Mon, Oct 14, 2024 at 01:36:46PM +0200, Anton Khirnov wrote:
>>>>> ---
>>>>>    fftools/opt_common.c | 4 +---
>>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/fftools/opt_common.c b/fftools/opt_common.c
>>>>> index 021ed75272..34da2cee7d 100644
>>>>> --- a/fftools/opt_common.c
>>>>> +++ b/fftools/opt_common.c
>>>>> @@ -808,7 +808,6 @@ int show_filters(void *optctx, const char *opt, const char *arg)
>>>>>        printf("Filters:\n"
>>>>>               "  T.. = Timeline support\n"
>>>>>               "  .S. = Slice threading\n"
>>>>> -           "  ..C = Command support\n"
>>>>>               "  A = Audio input/output\n"
>>>>>               "  V = Video input/output\n"
>>>>>               "  N = Dynamic number and/or type of input/output\n"
>>>>> @@ -833,10 +832,9 @@ int show_filters(void *optctx, const char *opt, const char *arg)
>>>>>                                      ( i && (filter->flags & AVFILTER_FLAG_DYNAMIC_OUTPUTS))) ? 'N' : '|';
>>>>>            }
>>>>>            *descr_cur = 0;
>>>>> -        printf(" %c%c%c %-17s %-10s %s\n",
>>>>> +        printf(" %c%c %-17s %-10s %s\n",
>>>>>                   filter->flags & AVFILTER_FLAG_SUPPORT_TIMELINE ? 'T' : '.',
>>>>>                   filter->flags & AVFILTER_FLAG_SLICE_THREADS    ? 'S' : '.',
>>>>> -               filter->process_command                        ? 'C' : '.',
>>>>>                   filter->name, descr, filter->description);
>>>>>        }
>>>>>    #else
>>>>
>>>> The commit message is not describing this change accurately
>>>>
>>>> Its not just "not accessing a priavte field", it removes information
>>>> from the printed filter list
>>>
>>> Learning question: How can we see this is a private field?
>>>
>>> This information was useful. How can we bring it back if we decide it's
>>> not OK too test if this field is NULL?
>>
>> A new flag AVFILTER_FLAG_SUPPORT_COMMANDS that the user can check to
>> ensure a call to avfilter_process_command() will not return ENOSYS could
>> be added. And of course, to print the C in here.
> 
> avfilter_process_command() may or may not return ENOSYS whether that
> flag is set or not, so I don't see what exactly would it be useful for.

I see, although I wouldn't expect ENOSYS for any other case than 
"unsupported". If a command fails, imo it would be EINVAL, or maybe ENOMEM.

> Same holds for printing the capability in fftools - just what is the
> user expected to do with that information?

Know they can use the send command interrupt for a given filter without 
having it unconditionally fail, i guess. But for that matter, is the 
list of supported commands available anywhere for the user? Is there an 
API to query them, or is it expected to be only in the documentation?
If there's no API, then maybe printing this is pointless.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20241015/1998cd06/attachment.sig>