diff mbox series

[FFmpeg-devel] ffmpeg: Make find_stream_info behave like a normal per-file option

Message ID 20220928102101.37967-1-epirat07@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel] ffmpeg: Make find_stream_info behave like a normal per-file option | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Marvin Scholz Sept. 28, 2022, 10:21 a.m. UTC 
Currently it would essentially change the find_stream_info setting for
the file it was specified for and all following files, which is unusual
and somewhat unexpected behaviour for a per-file option and not even
documented to behave like this.
---
 fftools/ffmpeg.h     | 1 +
 fftools/ffmpeg_opt.c | 7 ++++---
 2 files changed, 5 insertions(+), 3 deletions(-)

Comments

Anton Khirnov Oct. 12, 2022, 5:39 p.m. UTC | #1 
Quoting Marvin Scholz (2022-09-28 12:21:00)
> Currently it would essentially change the find_stream_info setting for
> the file it was specified for and all following files, which is unusual
> and somewhat unexpected behaviour for a per-file option and not even
> documented to behave like this.

Well, it's not documented at all.

> ---
>  fftools/ffmpeg.h     | 1 +
>  fftools/ffmpeg_opt.c | 7 ++++---
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index ede0b2bd96..3389ce5edc 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -126,6 +126,7 @@ typedef struct OptionsContext {
>      int accurate_seek;
>      int thread_queue_size;
>      int input_sync_ref;
> +    int find_stream_info;
>  
>      SpecifierOpt *ts_scale;
>      int        nb_ts_scale;
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 5febe319e4..df1c8df454 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -190,7 +190,6 @@ static int input_stream_potentially_available = 0;
>  static int ignore_unknown_streams = 0;
>  static int copy_unknown_streams = 0;
>  static int recast_media = 0;
> -static int find_stream_info = 1;
>  
>  static void uninit_options(OptionsContext *o)
>  {
> @@ -240,6 +239,7 @@ static void init_options(OptionsContext *o)
>      o->accurate_seek  = 1;
>      o->thread_queue_size = -1;
>      o->input_sync_ref = -1;
> +    o->find_stream_info = 1;
>      o->shortest_buf_duration = 10.f;
>  }
>  
> @@ -1307,7 +1307,7 @@ static int open_input_file(OptionsContext *o, const char *filename)
>      for (i = 0; i < ic->nb_streams; i++)
>          choose_decoder(o, ic, ic->streams[i], HWACCEL_NONE, AV_HWDEVICE_TYPE_NONE);
>  
> -    if (find_stream_info) {
> +    if (o->find_stream_info) {
>          AVDictionary **opts = setup_find_stream_info_opts(ic, o->g->codec_opts);
>          int orig_nb_streams = ic->nb_streams;
>  
> @@ -4000,7 +4000,8 @@ const OptionDef options[] = {
>      { "thread_queue_size", HAS_ARG | OPT_INT | OPT_OFFSET | OPT_EXPERT | OPT_INPUT | OPT_OUTPUT,
>                                                                       { .off = OFFSET(thread_queue_size) },
>          "set the maximum number of queued packets from the demuxer" },
> -    { "find_stream_info", OPT_BOOL | OPT_PERFILE | OPT_INPUT | OPT_EXPERT, { &find_stream_info },
> +    { "find_stream_info", OPT_BOOL | OPT_PERFILE | OPT_INPUT | OPT_EXPERT | OPT_OFFSET, 
> +                                                                     { .off = OFFSET(find_stream_info) },

You can drop OPT_PERFILE, since it's implied by OPT_OFFSET.

Otherwise patch LGTM, I'll do the change myself and push if you don't
object.
Marvin Scholz Oct. 12, 2022, 10:08 p.m. UTC | #2 
On 12 Oct 2022, at 19:39, Anton Khirnov wrote:

> Quoting Marvin Scholz (2022-09-28 12:21:00)
>> Currently it would essentially change the find_stream_info setting for
>> the file it was specified for and all following files, which is unusual
>> and somewhat unexpected behaviour for a per-file option and not even
>> documented to behave like this.
>
> Well, it's not documented at all.
>
>> ---
>>  fftools/ffmpeg.h     | 1 +
>>  fftools/ffmpeg_opt.c | 7 ++++---
>>  2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>> index ede0b2bd96..3389ce5edc 100644
>> --- a/fftools/ffmpeg.h
>> +++ b/fftools/ffmpeg.h
>> @@ -126,6 +126,7 @@ typedef struct OptionsContext {
>>      int accurate_seek;
>>      int thread_queue_size;
>>      int input_sync_ref;
>> +    int find_stream_info;
>>
>>      SpecifierOpt *ts_scale;
>>      int        nb_ts_scale;
>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>> index 5febe319e4..df1c8df454 100644
>> --- a/fftools/ffmpeg_opt.c
>> +++ b/fftools/ffmpeg_opt.c
>> @@ -190,7 +190,6 @@ static int input_stream_potentially_available = 0;
>>  static int ignore_unknown_streams = 0;
>>  static int copy_unknown_streams = 0;
>>  static int recast_media = 0;
>> -static int find_stream_info = 1;
>>
>>  static void uninit_options(OptionsContext *o)
>>  {
>> @@ -240,6 +239,7 @@ static void init_options(OptionsContext *o)
>>      o->accurate_seek  = 1;
>>      o->thread_queue_size = -1;
>>      o->input_sync_ref = -1;
>> +    o->find_stream_info = 1;
>>      o->shortest_buf_duration = 10.f;
>>  }
>>
>> @@ -1307,7 +1307,7 @@ static int open_input_file(OptionsContext *o, const char *filename)
>>      for (i = 0; i < ic->nb_streams; i++)
>>          choose_decoder(o, ic, ic->streams[i], HWACCEL_NONE, AV_HWDEVICE_TYPE_NONE);
>>
>> -    if (find_stream_info) {
>> +    if (o->find_stream_info) {
>>          AVDictionary **opts = setup_find_stream_info_opts(ic, o->g->codec_opts);
>>          int orig_nb_streams = ic->nb_streams;
>>
>> @@ -4000,7 +4000,8 @@ const OptionDef options[] = {
>>      { "thread_queue_size", HAS_ARG | OPT_INT | OPT_OFFSET | OPT_EXPERT | OPT_INPUT | OPT_OUTPUT,
>>                                                                       { .off = OFFSET(thread_queue_size) },
>>          "set the maximum number of queued packets from the demuxer" },
>> -    { "find_stream_info", OPT_BOOL | OPT_PERFILE | OPT_INPUT | OPT_EXPERT, { &find_stream_info },
>> +    { "find_stream_info", OPT_BOOL | OPT_PERFILE | OPT_INPUT | OPT_EXPERT | OPT_OFFSET,
>> +                                                                     { .off = OFFSET(find_stream_info) },
>
> You can drop OPT_PERFILE, since it's implied by OPT_OFFSET.
>
> Otherwise patch LGTM, I'll do the change myself and push if you don't
> object.
>

Thanks for the review, feel free to do the change yourself.

> -- 
> Anton Khirnov
diff mbox series

Patch

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index ede0b2bd96..3389ce5edc 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -126,6 +126,7 @@  typedef struct OptionsContext {
     int accurate_seek;
     int thread_queue_size;
     int input_sync_ref;
+    int find_stream_info;
 
     SpecifierOpt *ts_scale;
     int        nb_ts_scale;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 5febe319e4..df1c8df454 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -190,7 +190,6 @@  static int input_stream_potentially_available = 0;
 static int ignore_unknown_streams = 0;
 static int copy_unknown_streams = 0;
 static int recast_media = 0;
-static int find_stream_info = 1;
 
 static void uninit_options(OptionsContext *o)
 {
@@ -240,6 +239,7 @@  static void init_options(OptionsContext *o)
     o->accurate_seek  = 1;
     o->thread_queue_size = -1;
     o->input_sync_ref = -1;
+    o->find_stream_info = 1;
     o->shortest_buf_duration = 10.f;
 }
 
@@ -1307,7 +1307,7 @@  static int open_input_file(OptionsContext *o, const char *filename)
     for (i = 0; i < ic->nb_streams; i++)
         choose_decoder(o, ic, ic->streams[i], HWACCEL_NONE, AV_HWDEVICE_TYPE_NONE);
 
-    if (find_stream_info) {
+    if (o->find_stream_info) {
         AVDictionary **opts = setup_find_stream_info_opts(ic, o->g->codec_opts);
         int orig_nb_streams = ic->nb_streams;
 
@@ -4000,7 +4000,8 @@  const OptionDef options[] = {
     { "thread_queue_size", HAS_ARG | OPT_INT | OPT_OFFSET | OPT_EXPERT | OPT_INPUT | OPT_OUTPUT,
                                                                      { .off = OFFSET(thread_queue_size) },
         "set the maximum number of queued packets from the demuxer" },
-    { "find_stream_info", OPT_BOOL | OPT_PERFILE | OPT_INPUT | OPT_EXPERT, { &find_stream_info },
+    { "find_stream_info", OPT_BOOL | OPT_PERFILE | OPT_INPUT | OPT_EXPERT | OPT_OFFSET, 
+                                                                     { .off = OFFSET(find_stream_info) },
         "read and decode the streams to fill missing information with heuristics" },
     { "bits_per_raw_sample", OPT_INT | HAS_ARG | OPT_EXPERT | OPT_SPEC | OPT_OUTPUT,
         { .off = OFFSET(bits_per_raw_sample) },