| Message ID | 20240618190312.3848-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] ffprobe: always print all Stereo3D fields | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | fail | Make fate failed |
On 6/18/2024 4:03 PM, James Almer wrote: > ffprobe is meant to generate parseable output, and if a field is present, it > should be printed even if it has a default value. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > fftools/ffprobe.c | 9 +++------ > tests/ref/fate/matroska-spherical-mono | 3 +++ > tests/ref/fate/matroska-spherical-mono-remux | 6 ++++++ > tests/ref/fate/matroska-stereo_mode | 12 ++++++++++++ > tests/ref/fate/matroska-vp8-alpha-remux | 3 +++ > 5 files changed, 27 insertions(+), 6 deletions(-) > > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c > index a814cb5ade..d7ba980ff9 100644 > --- a/fftools/ffprobe.c > +++ b/fftools/ffprobe.c > @@ -2546,12 +2546,9 @@ static void print_pkt_side_data(WriterContext *w, > print_int("inverted", !!(stereo->flags & AV_STEREO3D_FLAG_INVERT)); > print_str("view", av_stereo3d_view_name(stereo->view)); > print_str("primary_eye", av_stereo3d_primary_eye_name(stereo->primary_eye)); > - if (stereo->baseline) > - print_int("baseline", stereo->baseline); > - if (stereo->horizontal_disparity_adjustment.num && stereo->horizontal_disparity_adjustment.den) > - print_q("horizontal_disparity_adjustment", stereo->horizontal_disparity_adjustment, '/'); > - if (stereo->horizontal_field_of_view) > - print_int("horizontal_field_of_view", stereo->horizontal_field_of_view); > + print_int("baseline", stereo->baseline); > + print_q("horizontal_disparity_adjustment", stereo->horizontal_disparity_adjustment, '/'); > + print_int("horizontal_field_of_view", stereo->horizontal_field_of_view); > } else if (sd->type == AV_PKT_DATA_SPHERICAL) { > const AVSphericalMapping *spherical = (AVSphericalMapping *)sd->data; > print_str("projection", av_spherical_projection_name(spherical->projection)); > diff --git a/tests/ref/fate/matroska-spherical-mono b/tests/ref/fate/matroska-spherical-mono > index 08b94e455b..c52ca8e7ee 100644 > --- a/tests/ref/fate/matroska-spherical-mono > +++ b/tests/ref/fate/matroska-spherical-mono > @@ -5,6 +5,9 @@ type=2D > inverted=0 > view=packed > primary_eye=none > +baseline=0 > +horizontal_disparity_adjustment=0/0 > +horizontal_field_of_view=0 > [/SIDE_DATA] > [SIDE_DATA] > side_data_type=Spherical Mapping > diff --git a/tests/ref/fate/matroska-spherical-mono-remux b/tests/ref/fate/matroska-spherical-mono-remux > index 0ca77c8074..10b92d5f2e 100644 > --- a/tests/ref/fate/matroska-spherical-mono-remux > +++ b/tests/ref/fate/matroska-spherical-mono-remux > @@ -29,6 +29,9 @@ type=2D > inverted=0 > view=packed > primary_eye=none > +baseline=0 > +horizontal_disparity_adjustment=0/0 > +horizontal_field_of_view=0 > [/SIDE_DATA] > [SIDE_DATA] > side_data_type=Spherical Mapping > @@ -55,6 +58,9 @@ type=2D > inverted=0 > view=packed > primary_eye=none > +baseline=0 > +horizontal_disparity_adjustment=0/0 > +horizontal_field_of_view=0 > [/SIDE_DATA] > [SIDE_DATA] > side_data_type=Spherical Mapping > diff --git a/tests/ref/fate/matroska-stereo_mode b/tests/ref/fate/matroska-stereo_mode > index 13bce13cb8..a1aab1e38e 100644 > --- a/tests/ref/fate/matroska-stereo_mode > +++ b/tests/ref/fate/matroska-stereo_mode > @@ -134,6 +134,9 @@ type=side by side > inverted=0 > view=packed > primary_eye=none > +baseline=0 > +horizontal_disparity_adjustment=0/0 > +horizontal_field_of_view=0 > [/SIDE_DATA] > [/STREAM] > [STREAM] > @@ -151,6 +154,9 @@ type=top and bottom > inverted=1 > view=packed > primary_eye=none > +baseline=0 > +horizontal_disparity_adjustment=0/0 > +horizontal_field_of_view=0 > [/SIDE_DATA] > [/STREAM] > [STREAM] > @@ -166,6 +172,9 @@ type=interleaved lines > inverted=1 > view=packed > primary_eye=none > +baseline=0 > +horizontal_disparity_adjustment=0/0 > +horizontal_field_of_view=0 > [/SIDE_DATA] > [/STREAM] > [STREAM] > @@ -182,6 +191,9 @@ type=interleaved columns > inverted=1 > view=packed > primary_eye=none > +baseline=0 > +horizontal_disparity_adjustment=0/0 > +horizontal_field_of_view=0 > [/SIDE_DATA] > [/STREAM] > [STREAM] > diff --git a/tests/ref/fate/matroska-vp8-alpha-remux b/tests/ref/fate/matroska-vp8-alpha-remux > index e54304cafd..ea8a089cec 100644 > --- a/tests/ref/fate/matroska-vp8-alpha-remux > +++ b/tests/ref/fate/matroska-vp8-alpha-remux > @@ -37,5 +37,8 @@ type=2D > inverted=0 > view=packed > primary_eye=none > +baseline=0 > +horizontal_disparity_adjustment=0/0 > +horizontal_field_of_view=0 > [/SIDE_DATA] > [/STREAM] Will apply soon if nobody objects.
Quoting James Almer (2024-06-18 21:03:12) > ffprobe is meant to generate parseable output, and if a field is present, it > should be printed even if it has a default value. This approach implies that adding new fields will change existing output, possibly just by updating the libraries without any changes in ffprobe itself. I was recently bitten by it when adding a new disposition, which suddenly breaks a number of completely unrelated FATE tests. I don't have strong opinions on how ffprobe Should Work(tm), but this result seems annoying in practice at least.
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c index a814cb5ade..d7ba980ff9 100644 --- a/fftools/ffprobe.c +++ b/fftools/ffprobe.c @@ -2546,12 +2546,9 @@ static void print_pkt_side_data(WriterContext *w, print_int("inverted", !!(stereo->flags & AV_STEREO3D_FLAG_INVERT)); print_str("view", av_stereo3d_view_name(stereo->view)); print_str("primary_eye", av_stereo3d_primary_eye_name(stereo->primary_eye)); - if (stereo->baseline) - print_int("baseline", stereo->baseline); - if (stereo->horizontal_disparity_adjustment.num && stereo->horizontal_disparity_adjustment.den) - print_q("horizontal_disparity_adjustment", stereo->horizontal_disparity_adjustment, '/'); - if (stereo->horizontal_field_of_view) - print_int("horizontal_field_of_view", stereo->horizontal_field_of_view); + print_int("baseline", stereo->baseline); + print_q("horizontal_disparity_adjustment", stereo->horizontal_disparity_adjustment, '/'); + print_int("horizontal_field_of_view", stereo->horizontal_field_of_view); } else if (sd->type == AV_PKT_DATA_SPHERICAL) { const AVSphericalMapping *spherical = (AVSphericalMapping *)sd->data; print_str("projection", av_spherical_projection_name(spherical->projection)); diff --git a/tests/ref/fate/matroska-spherical-mono b/tests/ref/fate/matroska-spherical-mono index 08b94e455b..c52ca8e7ee 100644 --- a/tests/ref/fate/matroska-spherical-mono +++ b/tests/ref/fate/matroska-spherical-mono @@ -5,6 +5,9 @@ type=2D inverted=0 view=packed primary_eye=none +baseline=0 +horizontal_disparity_adjustment=0/0 +horizontal_field_of_view=0 [/SIDE_DATA] [SIDE_DATA] side_data_type=Spherical Mapping diff --git a/tests/ref/fate/matroska-spherical-mono-remux b/tests/ref/fate/matroska-spherical-mono-remux index 0ca77c8074..10b92d5f2e 100644 --- a/tests/ref/fate/matroska-spherical-mono-remux +++ b/tests/ref/fate/matroska-spherical-mono-remux @@ -29,6 +29,9 @@ type=2D inverted=0 view=packed primary_eye=none +baseline=0 +horizontal_disparity_adjustment=0/0 +horizontal_field_of_view=0 [/SIDE_DATA] [SIDE_DATA] side_data_type=Spherical Mapping @@ -55,6 +58,9 @@ type=2D inverted=0 view=packed primary_eye=none +baseline=0 +horizontal_disparity_adjustment=0/0 +horizontal_field_of_view=0 [/SIDE_DATA] [SIDE_DATA] side_data_type=Spherical Mapping diff --git a/tests/ref/fate/matroska-stereo_mode b/tests/ref/fate/matroska-stereo_mode index 13bce13cb8..a1aab1e38e 100644 --- a/tests/ref/fate/matroska-stereo_mode +++ b/tests/ref/fate/matroska-stereo_mode @@ -134,6 +134,9 @@ type=side by side inverted=0 view=packed primary_eye=none +baseline=0 +horizontal_disparity_adjustment=0/0 +horizontal_field_of_view=0 [/SIDE_DATA] [/STREAM] [STREAM] @@ -151,6 +154,9 @@ type=top and bottom inverted=1 view=packed primary_eye=none +baseline=0 +horizontal_disparity_adjustment=0/0 +horizontal_field_of_view=0 [/SIDE_DATA] [/STREAM] [STREAM] @@ -166,6 +172,9 @@ type=interleaved lines inverted=1 view=packed primary_eye=none +baseline=0 +horizontal_disparity_adjustment=0/0 +horizontal_field_of_view=0 [/SIDE_DATA] [/STREAM] [STREAM] @@ -182,6 +191,9 @@ type=interleaved columns inverted=1 view=packed primary_eye=none +baseline=0 +horizontal_disparity_adjustment=0/0 +horizontal_field_of_view=0 [/SIDE_DATA] [/STREAM] [STREAM] diff --git a/tests/ref/fate/matroska-vp8-alpha-remux b/tests/ref/fate/matroska-vp8-alpha-remux index e54304cafd..ea8a089cec 100644 --- a/tests/ref/fate/matroska-vp8-alpha-remux +++ b/tests/ref/fate/matroska-vp8-alpha-remux @@ -37,5 +37,8 @@ type=2D inverted=0 view=packed primary_eye=none +baseline=0 +horizontal_disparity_adjustment=0/0 +horizontal_field_of_view=0 [/SIDE_DATA] [/STREAM]
ffprobe is meant to generate parseable output, and if a field is present, it should be printed even if it has a default value. Signed-off-by: James Almer <jamrial@gmail.com> --- fftools/ffprobe.c | 9 +++------ tests/ref/fate/matroska-spherical-mono | 3 +++ tests/ref/fate/matroska-spherical-mono-remux | 6 ++++++ tests/ref/fate/matroska-stereo_mode | 12 ++++++++++++ tests/ref/fate/matroska-vp8-alpha-remux | 3 +++ 5 files changed, 27 insertions(+), 6 deletions(-)