diff mbox series

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

Message ID 20220922132023.240086-1-derek.buitenhuis@gmail.com
State New
Headers show
Series [FFmpeg-devel] ffprobe: Check for invalid matrix error when printing rotation | expand

Checks

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

Commit Message

Derek Buitenhuis Sept. 22, 2022, 1:20 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.
---
 fftools/ffprobe.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Derek Buitenhuis Sept. 22, 2022, 2:01 p.m. UTC | #1 
On 9/22/2022 2:20 PM, Derek Buitenhuis wrote:
> ---
>  fftools/ffprobe.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

Sorry, broken patch. Will send v2.

- Derek
diff mbox series

Patch

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 8ee1b1036c..915ed0fb9c 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -2282,8 +2282,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 (rotation == NAN)
+                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", 0);
         } 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 +2642,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 (rotation == NAN)
+                    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) {