[FFmpeg-devel] [PATCH] avfilter/af_afade: fix opt_type for nb_samples/ns

[FFmpeg-devel] [PATCH] avfilter/af_afade: fix opt_type for nb_samples/ns

[Andrew Sayers]

The actual value is an int64_t, and is accessed elsewhere as AV_OPT_TYPE_INT64.

Accessing it as INT will likely cause bugs on some 32-bit architectures.
---
 libavfilter/af_afade.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavfilter/af_afade.c b/libavfilter/af_afade.c
index 3a45873460..c79271ec92 100644
--- a/libavfilter/af_afade.c
+++ b/libavfilter/af_afade.c
@@ -452,8 +452,8 @@ const AVFilter ff_af_afade = {
 #if CONFIG_ACROSSFADE_FILTER
 
 static const AVOption acrossfade_options[] = {
-    { "nb_samples",   "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT,    {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
-    { "ns",           "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT,    {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
+    { "nb_samples",   "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT64,  {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
+    { "ns",           "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT64,  {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
     { "duration",     "set cross fade duration",                       OFFSET(duration),     AV_OPT_TYPE_DURATION, {.i64 = 0 },  0, 60000000, FLAGS },
     { "d",            "set cross fade duration",                       OFFSET(duration),     AV_OPT_TYPE_DURATION, {.i64 = 0 },  0, 60000000, FLAGS },
     { "overlap",      "overlap 1st stream end with 2nd stream start",  OFFSET(overlap),      AV_OPT_TYPE_BOOL,   {.i64 = 1    }, 0,  1, FLAGS },
-- 
2.45.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/af_afade: fix opt_type for nb_samples/ns

[Andreas Rheinhardt]

Andrew Sayers:
> The actual value is an int64_t, and is accessed elsewhere as AV_OPT_TYPE_INT64.
> 
> Accessing it as INT will likely cause bugs on some 32-bit architectures.

Whether this works or not will depend upon endianness, not on whether
the architecture is 32-bit (as long as int is 32bits, which is mostly
true for 64-bit architectures).

> ---
>  libavfilter/af_afade.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavfilter/af_afade.c b/libavfilter/af_afade.c
> index 3a45873460..c79271ec92 100644
> --- a/libavfilter/af_afade.c
> +++ b/libavfilter/af_afade.c
> @@ -452,8 +452,8 @@ const AVFilter ff_af_afade = {
>  #if CONFIG_ACROSSFADE_FILTER
>  
>  static const AVOption acrossfade_options[] = {
> -    { "nb_samples",   "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT,    {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
> -    { "ns",           "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT,    {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
> +    { "nb_samples",   "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT64,  {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
> +    { "ns",           "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT64,  {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
>      { "duration",     "set cross fade duration",                       OFFSET(duration),     AV_OPT_TYPE_DURATION, {.i64 = 0 },  0, 60000000, FLAGS },
>      { "d",            "set cross fade duration",                       OFFSET(duration),     AV_OPT_TYPE_DURATION, {.i64 = 0 },  0, 60000000, FLAGS },
>      { "overlap",      "overlap 1st stream end with 2nd stream start",  OFFSET(overlap),      AV_OPT_TYPE_BOOL,   {.i64 = 1    }, 0,  1, FLAGS },

LGTM. How did you find this?

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/af_afade: fix opt_type for nb_samples/ns

[Andrew Sayers]

On Sat, Jun 22, 2024 at 03:20:52PM +0200, Andreas Rheinhardt wrote:
> Andrew Sayers:
> > The actual value is an int64_t, and is accessed elsewhere as AV_OPT_TYPE_INT64.
> > 
> > Accessing it as INT will likely cause bugs on some 32-bit architectures.
> 
> Whether this works or not will depend upon endianness, not on whether
> the architecture is 32-bit (as long as int is 32bits, which is mostly
> true for 64-bit architectures).
> 
> > ---
> >  libavfilter/af_afade.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavfilter/af_afade.c b/libavfilter/af_afade.c
> > index 3a45873460..c79271ec92 100644
> > --- a/libavfilter/af_afade.c
> > +++ b/libavfilter/af_afade.c
> > @@ -452,8 +452,8 @@ const AVFilter ff_af_afade = {
> >  #if CONFIG_ACROSSFADE_FILTER
> >  
> >  static const AVOption acrossfade_options[] = {
> > -    { "nb_samples",   "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT,    {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
> > -    { "ns",           "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT,    {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
> > +    { "nb_samples",   "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT64,  {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
> > +    { "ns",           "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT64,  {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
> >      { "duration",     "set cross fade duration",                       OFFSET(duration),     AV_OPT_TYPE_DURATION, {.i64 = 0 },  0, 60000000, FLAGS },
> >      { "d",            "set cross fade duration",                       OFFSET(duration),     AV_OPT_TYPE_DURATION, {.i64 = 0 },  0, 60000000, FLAGS },
> >      { "overlap",      "overlap 1st stream end with 2nd stream start",  OFFSET(overlap),      AV_OPT_TYPE_BOOL,   {.i64 = 1    }, 0,  1, FLAGS },
> 
> LGTM. How did you find this?

It was a side-effect of yet another attempt to understand the codebase -
looking for struct members with more than one associated AVOptionType.

FWIW, the other oddities uncovered were:

* libavformat/rtsp.c treats stimeout as both INT64 and DURATION
  (should probably have been caught at the time, but fixing it would likely
  break scripts that expect the current CLI behaviour)
* libavdevice/v4l2.c treats list_format as both INT and CONST
  (not a bug, but a comment explaining this clever trick would have been nice)
* libavfilter/buffersrc.c lets you set w/h separately or as video_size
  (not a bug, and I assume there's a reason for doing it this way)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/af_afade: fix opt_type for nb_samples/ns

[Stefano Sabatini]

On date Saturday 2024-06-22 15:20:52 +0200, Andreas Rheinhardt wrote:
> Andrew Sayers:
> > The actual value is an int64_t, and is accessed elsewhere as AV_OPT_TYPE_INT64.
> > 
> > Accessing it as INT will likely cause bugs on some 32-bit architectures.
> 
> Whether this works or not will depend upon endianness, not on whether
> the architecture is 32-bit (as long as int is 32bits, which is mostly
> true for 64-bit architectures).
>
> > ---
> >  libavfilter/af_afade.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavfilter/af_afade.c b/libavfilter/af_afade.c
> > index 3a45873460..c79271ec92 100644
> > --- a/libavfilter/af_afade.c
> > +++ b/libavfilter/af_afade.c
> > @@ -452,8 +452,8 @@ const AVFilter ff_af_afade = {
> >  #if CONFIG_ACROSSFADE_FILTER
> >  
> >  static const AVOption acrossfade_options[] = {
> > -    { "nb_samples",   "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT,    {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
> > -    { "ns",           "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT,    {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
> > +    { "nb_samples",   "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT64,  {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
> > +    { "ns",           "set number of samples for cross fade duration", OFFSET(nb_samples),   AV_OPT_TYPE_INT64,  {.i64 = 44100}, 1, INT32_MAX/10, FLAGS },
> >      { "duration",     "set cross fade duration",                       OFFSET(duration),     AV_OPT_TYPE_DURATION, {.i64 = 0 },  0, 60000000, FLAGS },
> >      { "d",            "set cross fade duration",                       OFFSET(duration),     AV_OPT_TYPE_DURATION, {.i64 = 0 },  0, 60000000, FLAGS },
> >      { "overlap",      "overlap 1st stream end with 2nd stream start",  OFFSET(overlap),      AV_OPT_TYPE_BOOL,   {.i64 = 1    }, 0,  1, FLAGS },
> 
> LGTM. How did you find this?

LGTM as well, will apply after dropping the second sentence in the
commit log.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".