diff mbox series

[FFmpeg-devel,v2] ffprobe: Check for invalid matrix error when printing rotation

Message ID 20220922140800.240449-1-derek.buitenhuis@gmail.com
State Accepted
Commit f5cd00bf5265cb1d10de925c4d0b3ebb191f0d74
Headers show
Series [FFmpeg-devel,v2] ffprobe: Check for invalid matrix error when printing rotation | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Derek Buitenhuis Sept. 22, 2022, 2:08 p.m. UTC 
av_display_rotation_get will return NAN when the display matrix is invalid,
which would end up printing NAN as an integer in the rotation field. This
is poor for multiple reasons:

* Users of ffprobe have no way of discerning "valid but ugly rotation from
  display matrix" from "invalid display matrix".
* It can have unintended consequences on some platforms, such as Linux x86_64,
  where NAN is equal to INT64_MIN, which, for example, when printed as JSON,
  which uses floating point for all numbers, can end up as invalid JSON or wit
  a number that cannot be reserialized as an integer at all.

Since NAN is av_display_rotation_get's error case, just print 0 (no rotation)
when that happens.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
I could have also changed av_display_rotation_get to return 0, but I didn't
want to make an API change.

Arguably, though, returning NAN on error isn't an amazing API to begin with.

v2 now with typo fix and proper NAN checks.
---
 fftools/ffprobe.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Derek Buitenhuis Sept. 25, 2022, 11:10 a.m. UTC | #1 
On 9/22/2022 3:08 PM, Derek Buitenhuis wrote:
> ---
>  fftools/ffprobe.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)

Will push later today (with commit message typo fixed)
if there are no objections.

- Derek
diff mbox series

Patch

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 8ee1b1036c..421ada5bd8 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -27,6 +27,7 @@ 
 #include "libavutil/ffversion.h"
 
 #include <string.h>
+#include <math.h>
 
 #include "libavformat/avformat.h"
 #include "libavformat/version.h"
@@ -2282,8 +2283,11 @@  static void print_pkt_side_data(WriterContext *w,
         writer_print_section_header(w, id_data);
         print_str("side_data_type", name ? name : "unknown");
         if (sd->type == AV_PKT_DATA_DISPLAYMATRIX && sd->size >= 9*4) {
+            double rotation = av_display_rotation_get((int32_t *)sd->data);
+            if (isnan(rotation))
+                rotation = 0;
             writer_print_integers(w, "displaymatrix", sd->data, 9, " %11d", 3, 4, 1);
-            print_int("rotation", av_display_rotation_get((int32_t *)sd->data));
+            print_int("rotation", rotation);
         } else if (sd->type == AV_PKT_DATA_STEREO3D) {
             const AVStereo3D *stereo = (AVStereo3D *)sd->data;
             print_str("type", av_stereo3d_type_name(stereo->type));
@@ -2639,8 +2643,11 @@  static void show_frame(WriterContext *w, AVFrame *frame, AVStream *stream,
             name = av_frame_side_data_name(sd->type);
             print_str("side_data_type", name ? name : "unknown");
             if (sd->type == AV_FRAME_DATA_DISPLAYMATRIX && sd->size >= 9*4) {
+                double rotation = av_display_rotation_get((int32_t *)sd->data);
+                if (isnan(rotation))
+                    rotation = 0;
                 writer_print_integers(w, "displaymatrix", sd->data, 9, " %11d", 3, 4, 1);
-                print_int("rotation", av_display_rotation_get((int32_t *)sd->data));
+                print_int("rotation", rotation);
             } else if (sd->type == AV_FRAME_DATA_AFD && sd->size > 0) {
                 print_int("active_format", *sd->data);
             } else if (sd->type == AV_FRAME_DATA_GOP_TIMECODE && sd->size >= 8) {