[FFmpeg-devel,01/14] avutil/opt: add AV_OPT_FLAG_RUNTIME_PARAM flag

Submitted by Paul B Mahol on Oct. 10, 2019, 11:38 a.m.

Details

Message ID 20191010113851.27196-1-onemda@gmail.com
State New
Headers show

Commit Message

Paul B Mahol Oct. 10, 2019, 11:38 a.m.
Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavutil/opt.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Nicolas George Oct. 13, 2019, 5:34 p.m.
Paul B Mahol (12019-10-10):
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavutil/opt.h | 1 +
>  1 file changed, 1 insertion(+)

The patch series looks reasonable on the whole. It changes the return
code of process_command() in a few cases, but that should not have
consequences.

But it is completely missing the user documentation. I suggest to add a
paragraph like that at the appropriate place:

    Changing options at runtime with a command.

    Some options can be changed during the operation of the filter using
    a command. These options are marked 'R' on the output of `ffmpeg -h
    filter=...`. The name of the command is the name of the option and
    the argument is the new value.

Then, for each option newly marked with this flag:

    This option can be changed at runtime with a command. See *Changing
    options at runtime with a command.* for details.

I think that should be enough.

Regards,
Paul B Mahol Oct. 13, 2019, 5:46 p.m.
On 10/13/19, Nicolas George <george@nsup.org> wrote:
> Paul B Mahol (12019-10-10):
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavutil/opt.h | 1 +
>>  1 file changed, 1 insertion(+)
>
> The patch series looks reasonable on the whole. It changes the return
> code of process_command() in a few cases, but that should not have
> consequences.
>
> But it is completely missing the user documentation. I suggest to add a
> paragraph like that at the appropriate place:
>
>     Changing options at runtime with a command.
>
>     Some options can be changed during the operation of the filter using
>     a command. These options are marked 'R' on the output of `ffmpeg -h
>     filter=...`. The name of the command is the name of the option and
>     the argument is the new value.

R is already taken for realtime option.
Another patch uses T for this.

>
> Then, for each option newly marked with this flag:
>
>     This option can be changed at runtime with a command. See *Changing
>     options at runtime with a command.* for details.
>
> I think that should be enough.
>
> Regards,
>
> --
>   Nicolas George
>
Nicolas George Oct. 13, 2019, 5:55 p.m.
Paul B Mahol (12019-10-13):
> R is already taken for realtime option.
> Another patch uses T for this.

I meant the code you did use in the other patch, I neglected to check.

Patch hide | download patch | download mbox

diff --git a/libavutil/opt.h b/libavutil/opt.h
index 39f4a8dda0..bc98ab104d 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -288,6 +288,7 @@  typedef struct AVOption {
  */
 #define AV_OPT_FLAG_READONLY        128
 #define AV_OPT_FLAG_BSF_PARAM       (1<<8) ///< a generic parameter which can be set by the user for bit stream filtering
+#define AV_OPT_FLAG_RUNTIME_PARAM   (1<<15) ///< a generic parameter which can be set by the user at runtime
 #define AV_OPT_FLAG_FILTERING_PARAM (1<<16) ///< a generic parameter which can be set by the user for filtering
 #define AV_OPT_FLAG_DEPRECATED      (1<<17) ///< set if option is deprecated, users should refer to AVOption.help text for more information
 //FIXME think about enc-audio, ... style flags