[FFmpeg-devel] [PATCH 1/3] avutil/ambient_viewing_environment: set a sane default value for AVRational fields

[FFmpeg-devel] [PATCH 1/3] avutil/ambient_viewing_environment: set a sane default value for AVRational fields

[James Almer]

Prevent potential divisions by 0 when using them immediately after allocation.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavutil/ambient_viewing_environment.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/libavutil/ambient_viewing_environment.c b/libavutil/ambient_viewing_environment.c
index c47458cfa8..e359727776 100644
--- a/libavutil/ambient_viewing_environment.c
+++ b/libavutil/ambient_viewing_environment.c
@@ -21,6 +21,13 @@
 #include "ambient_viewing_environment.h"
 #include "mem.h"
 
+static void get_defaults(AVAmbientViewingEnvironment *env)
+{
+    env->ambient_illuminance =
+    env->ambient_light_x     =
+    env->ambient_light_y     = (AVRational) { 0, 1 };
+}
+
 AVAmbientViewingEnvironment *av_ambient_viewing_environment_alloc(size_t *size)
 {
     AVAmbientViewingEnvironment *env =
@@ -28,6 +35,8 @@ AVAmbientViewingEnvironment *av_ambient_viewing_environment_alloc(size_t *size)
     if (!env)
         return NULL;
 
+    get_defaults(env);
+
      if (size)
         *size = sizeof(*env);
 
@@ -44,6 +53,7 @@ AVAmbientViewingEnvironment *av_ambient_viewing_environment_create_side_data(AVF
         return NULL;
 
     memset(side_data->data, 0, side_data->size);
+    get_defaults((AVAmbientViewingEnvironment *)side_data->data);
 
     return (AVAmbientViewingEnvironment *)side_data->data;
 }
-- 
2.45.2

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/3] avutil/mastering_display_metadata: set a sane default value for AVRational fields

[James Almer]

Prevent potential divisions by 0 when using them immediately after allocation.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavutil/mastering_display_metadata.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/libavutil/mastering_display_metadata.c b/libavutil/mastering_display_metadata.c
index ea41f13f9d..73a36c661e 100644
--- a/libavutil/mastering_display_metadata.c
+++ b/libavutil/mastering_display_metadata.c
@@ -25,9 +25,15 @@
 #include "mastering_display_metadata.h"
 #include "mem.h"
 
-AVMasteringDisplayMetadata *av_mastering_display_metadata_alloc(void)
+static void get_defaults(AVMasteringDisplayMetadata *mastering)
 {
-    return av_mallocz(sizeof(AVMasteringDisplayMetadata));
+    for (int i = 0; i < 3; i++)
+        for (int j = 0; j < 2; j++)
+            mastering->display_primaries[i][j] = (AVRational) { 0, 1 };
+    mastering->white_point[0] =
+    mastering->white_point[1] =
+    mastering->min_luminance  =
+    mastering->max_luminance  = (AVRational) { 0, 1 };
 }
 
 AVMasteringDisplayMetadata *av_mastering_display_metadata_alloc_size(size_t *size)
@@ -36,12 +42,19 @@ AVMasteringDisplayMetadata *av_mastering_display_metadata_alloc_size(size_t *siz
     if (!mastering)
         return NULL;
 
+    get_defaults(mastering);
+
     if (size)
         *size = sizeof(*mastering);
 
     return mastering;
 }
 
+AVMasteringDisplayMetadata *av_mastering_display_metadata_alloc(void)
+{
+    return av_mastering_display_metadata_alloc_size(NULL);
+}
+
 AVMasteringDisplayMetadata *av_mastering_display_metadata_create_side_data(AVFrame *frame)
 {
     AVFrameSideData *side_data = av_frame_new_side_data(frame,
@@ -51,6 +64,7 @@ AVMasteringDisplayMetadata *av_mastering_display_metadata_create_side_data(AVFra
         return NULL;
 
     memset(side_data->data, 0, sizeof(AVMasteringDisplayMetadata));
+    get_defaults((AVMasteringDisplayMetadata *)side_data->data);
 
     return (AVMasteringDisplayMetadata *)side_data->data;
 }
-- 
2.45.2

_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/3] avutil/stereo3d: set a sane default value for AVRational fields

[James Almer]

Prevent potential divisions by 0 when using them immediately after allocation.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavutil/stereo3d.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/libavutil/stereo3d.c b/libavutil/stereo3d.c
index a40a9439bb..19e81e4124 100644
--- a/libavutil/stereo3d.c
+++ b/libavutil/stereo3d.c
@@ -26,9 +26,20 @@
 #include "mem.h"
 #include "stereo3d.h"
 
+static void get_defaults(AVStereo3D *stereo)
+{
+    stereo->horizontal_disparity_adjustment = (AVRational) { 0, 1 };
+}
+
 AVStereo3D *av_stereo3d_alloc(void)
 {
-    return av_mallocz(sizeof(AVStereo3D));
+    AVStereo3D *stereo = av_mallocz(sizeof(AVStereo3D));
+    if (!stereo)
+        return NULL;
+
+    get_defaults(stereo);
+
+    return stereo;
 }
 
 AVStereo3D *av_stereo3d_create_side_data(AVFrame *frame)
@@ -40,6 +51,7 @@ AVStereo3D *av_stereo3d_create_side_data(AVFrame *frame)
         return NULL;
 
     memset(side_data->data, 0, sizeof(AVStereo3D));
+    get_defaults((AVStereo3D *)side_data->data);
 
     return (AVStereo3D *)side_data->data;
 }
-- 
2.45.2

_______________________________________________
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 3/3] avutil/stereo3d: set a sane default value for AVRational fields

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1039 bytes --]

On Tue, Jun 18, 2024 at 04:20:34PM -0300, James Almer wrote:
> Prevent potential divisions by 0 when using them immediately after allocation.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavutil/stereo3d.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)

i must have applied the wrong patches locally but this fails to build

CC	libavutil/stereo3d.o
libavutil/stereo3d.c: In function ‘get_defaults’:
libavutil/stereo3d.c:31:47: error: incompatible types when assigning to type ‘int32_t’ {aka ‘int’} from type ‘AVRational’ {aka ‘struct AVRational’}
   31 |     stereo->horizontal_disparity_adjustment = (AVRational) { 0, 1 };
      |                                               ^
make: *** [ffbuild/common.mak:81: libavutil/stereo3d.o] Error 1

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 3/3] avutil/stereo3d: set a sane default value for AVRational fields

[James Almer]

On 6/18/2024 6:13 PM, Michael Niedermayer wrote:
> On Tue, Jun 18, 2024 at 04:20:34PM -0300, James Almer wrote:
>> Prevent potential divisions by 0 when using them immediately after allocation.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavutil/stereo3d.c | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
> 
> i must have applied the wrong patches locally but this fails to build
> 
> CC	libavutil/stereo3d.o
> libavutil/stereo3d.c: In function ‘get_defaults’:
> libavutil/stereo3d.c:31:47: error: incompatible types when assigning to type ‘int32_t’ {aka ‘int’} from type ‘AVRational’ {aka ‘struct AVRational’}

This sounds like you still have Derek's old version in your tree. He has 
since changed it and pushed a different one.

>     31 |     stereo->horizontal_disparity_adjustment = (AVRational) { 0, 1 };
>        |                                               ^
> make: *** [ffbuild/common.mak:81: libavutil/stereo3d.o] Error 1
> 
> thx
> 
> [...]
> 
> 
> _______________________________________________
> 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".
_______________________________________________
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 3/3] avutil/stereo3d: set a sane default value for AVRational fields

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1276 bytes --]

On Fri, Jun 21, 2024 at 09:19:36AM -0300, James Almer wrote:
> On 6/18/2024 6:13 PM, Michael Niedermayer wrote:
> > On Tue, Jun 18, 2024 at 04:20:34PM -0300, James Almer wrote:
> > > Prevent potential divisions by 0 when using them immediately after allocation.
> > > 
> > > Signed-off-by: James Almer <jamrial@gmail.com>
> > > ---
> > >   libavutil/stereo3d.c | 14 +++++++++++++-
> > >   1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > i must have applied the wrong patches locally but this fails to build
> > 
> > CC	libavutil/stereo3d.o
> > libavutil/stereo3d.c: In function ‘get_defaults’:
> > libavutil/stereo3d.c:31:47: error: incompatible types when assigning to type ‘int32_t’ {aka ‘int’} from type ‘AVRational’ {aka ‘struct AVRational’}
> 
> This sounds like you still have Derek's old version in your tree. He has
> since changed it and pushed a different one.

yes, i realized and fixed that after i sent the mail

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 1/3] avutil/ambient_viewing_environment: set a sane default value for AVRational fields

[James Almer]

On 6/18/2024 4:20 PM, James Almer wrote:
> Prevent potential divisions by 0 when using them immediately after allocation.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>   libavutil/ambient_viewing_environment.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/libavutil/ambient_viewing_environment.c b/libavutil/ambient_viewing_environment.c
> index c47458cfa8..e359727776 100644
> --- a/libavutil/ambient_viewing_environment.c
> +++ b/libavutil/ambient_viewing_environment.c
> @@ -21,6 +21,13 @@
>   #include "ambient_viewing_environment.h"
>   #include "mem.h"
>   
> +static void get_defaults(AVAmbientViewingEnvironment *env)
> +{
> +    env->ambient_illuminance =
> +    env->ambient_light_x     =
> +    env->ambient_light_y     = (AVRational) { 0, 1 };
> +}
> +
>   AVAmbientViewingEnvironment *av_ambient_viewing_environment_alloc(size_t *size)
>   {
>       AVAmbientViewingEnvironment *env =
> @@ -28,6 +35,8 @@ AVAmbientViewingEnvironment *av_ambient_viewing_environment_alloc(size_t *size)
>       if (!env)
>           return NULL;
>   
> +    get_defaults(env);
> +
>        if (size)
>           *size = sizeof(*env);
>   
> @@ -44,6 +53,7 @@ AVAmbientViewingEnvironment *av_ambient_viewing_environment_create_side_data(AVF
>           return NULL;
>   
>       memset(side_data->data, 0, side_data->size);
> +    get_defaults((AVAmbientViewingEnvironment *)side_data->data);
>   
>       return (AVAmbientViewingEnvironment *)side_data->data;
>   }

Will apply the set soon if nobody objectx.
_______________________________________________
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 1/3] avutil/ambient_viewing_environment: set a sane default value for AVRational fields

[Jan Ekström]

On Thu, Jun 20, 2024 at 6:54 PM James Almer <jamrial@gmail.com> wrote:
>
> On 6/18/2024 4:20 PM, James Almer wrote:
> > Prevent potential divisions by 0 when using them immediately after allocation.
> >
> > Signed-off-by: James Almer <jamrial@gmail.com>
> > ---
> >   libavutil/ambient_viewing_environment.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> >
> > diff --git a/libavutil/ambient_viewing_environment.c b/libavutil/ambient_viewing_environment.c
> > index c47458cfa8..e359727776 100644
> > --- a/libavutil/ambient_viewing_environment.c
> > +++ b/libavutil/ambient_viewing_environment.c
> > @@ -21,6 +21,13 @@
> >   #include "ambient_viewing_environment.h"
> >   #include "mem.h"
> >
> > +static void get_defaults(AVAmbientViewingEnvironment *env)
> > +{
> > +    env->ambient_illuminance =
> > +    env->ambient_light_x     =
> > +    env->ambient_light_y     = (AVRational) { 0, 1 };
> > +}
> > +
> >   AVAmbientViewingEnvironment *av_ambient_viewing_environment_alloc(size_t *size)
> >   {
> >       AVAmbientViewingEnvironment *env =
> > @@ -28,6 +35,8 @@ AVAmbientViewingEnvironment *av_ambient_viewing_environment_alloc(size_t *size)
> >       if (!env)
> >           return NULL;
> >
> > +    get_defaults(env);
> > +
> >        if (size)
> >           *size = sizeof(*env);
> >
> > @@ -44,6 +53,7 @@ AVAmbientViewingEnvironment *av_ambient_viewing_environment_create_side_data(AVF
> >           return NULL;
> >
> >       memset(side_data->data, 0, side_data->size);
> > +    get_defaults((AVAmbientViewingEnvironment *)side_data->data);
> >
> >       return (AVAmbientViewingEnvironment *)side_data->data;
> >   }
>
> Will apply the set soon if nobody objectx.

In general I guess LGTM as I guess 0/0 is considered bad.

But isn't the function more about setting the defaults to a struct
rather than getting them? I just see that function being closer to
"set_defaults" than "get_defaults".

Jan
_______________________________________________
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".