diff mbox

[FFmpeg-devel,v2] ffprobe: use consistent string for unspecified color_range value

Message ID 1504015658-27989-1-git-send-email-t.rapp@noa-archive.com
State New
Headers show

Commit Message

Tobias Rapp Aug. 29, 2017, 2:07 p.m. UTC
Makes the handling of unspecified/unknown color_range values on stream
level consistent to the value used on frame level.

Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com>
---
 ffprobe.c                      | 8 ++++----
 tests/ref/fate/ffprobe_compact | 4 ++--
 tests/ref/fate/ffprobe_csv     | 4 ++--
 tests/ref/fate/ffprobe_default | 4 ++--
 tests/ref/fate/ffprobe_flat    | 4 ++--
 tests/ref/fate/ffprobe_ini     | 4 ++--
 tests/ref/fate/mxf-probe-dnxhd | 2 +-
 tests/ref/fate/mxf-probe-dv25  | 2 +-
 8 files changed, 16 insertions(+), 16 deletions(-)

Comments

Tobias Rapp Sept. 5, 2017, 12:50 p.m. UTC | #1
On 29.08.2017 16:07, Tobias Rapp wrote:
> Makes the handling of unspecified/unknown color_range values on stream
> level consistent to the value used on frame level.
> 
> Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com>
> ---
>   ffprobe.c                      | 8 ++++----
>   tests/ref/fate/ffprobe_compact | 4 ++--
>   tests/ref/fate/ffprobe_csv     | 4 ++--
>   tests/ref/fate/ffprobe_default | 4 ++--
>   tests/ref/fate/ffprobe_flat    | 4 ++--
>   tests/ref/fate/ffprobe_ini     | 4 ++--
>   tests/ref/fate/mxf-probe-dnxhd | 2 +-
>   tests/ref/fate/mxf-probe-dv25  | 2 +-
>   8 files changed, 16 insertions(+), 16 deletions(-)
> [...]

I'd like to push this if there are no objections within the next two days.

Regards,
Tobias
Paul B Mahol Sept. 5, 2017, 1:28 p.m. UTC | #2
On 9/5/17, Tobias Rapp <t.rapp@noa-archive.com> wrote:
> On 29.08.2017 16:07, Tobias Rapp wrote:
>> Makes the handling of unspecified/unknown color_range values on stream
>> level consistent to the value used on frame level.
>>
>> Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com>
>> ---
>>   ffprobe.c                      | 8 ++++----
>>   tests/ref/fate/ffprobe_compact | 4 ++--
>>   tests/ref/fate/ffprobe_csv     | 4 ++--
>>   tests/ref/fate/ffprobe_default | 4 ++--
>>   tests/ref/fate/ffprobe_flat    | 4 ++--
>>   tests/ref/fate/ffprobe_ini     | 4 ++--
>>   tests/ref/fate/mxf-probe-dnxhd | 2 +-
>>   tests/ref/fate/mxf-probe-dv25  | 2 +-
>>   8 files changed, 16 insertions(+), 16 deletions(-)
>> [...]
>
> I'd like to push this if there are no objections within the next two days.
>

LGTM
Tobias Rapp Sept. 7, 2017, 11:31 a.m. UTC | #3
On 05.09.2017 15:28, Paul B Mahol wrote:
> On 9/5/17, Tobias Rapp <t.rapp@noa-archive.com> wrote:
>> On 29.08.2017 16:07, Tobias Rapp wrote:
>>> Makes the handling of unspecified/unknown color_range values on stream
>>> level consistent to the value used on frame level.
>>>
>>> Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com>
>>> ---
>>>    ffprobe.c                      | 8 ++++----
>>>    tests/ref/fate/ffprobe_compact | 4 ++--
>>>    tests/ref/fate/ffprobe_csv     | 4 ++--
>>>    tests/ref/fate/ffprobe_default | 4 ++--
>>>    tests/ref/fate/ffprobe_flat    | 4 ++--
>>>    tests/ref/fate/ffprobe_ini     | 4 ++--
>>>    tests/ref/fate/mxf-probe-dnxhd | 2 +-
>>>    tests/ref/fate/mxf-probe-dv25  | 2 +-
>>>    8 files changed, 16 insertions(+), 16 deletions(-)
>>> [...]
>>
>> I'd like to push this if there are no objections within the next two days.
>>
> 
> LGTM

Pushed, thanks for review.

Regards,
Tobias
diff mbox

Patch

diff --git a/ffprobe.c b/ffprobe.c
index ba10563..b2e8949 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -1925,11 +1925,11 @@  static void print_pkt_side_data(WriterContext *w,
     writer_print_section_footer(w);
 }
 
-static void print_color_range(WriterContext *w, enum AVColorRange color_range, const char *fallback)
+static void print_color_range(WriterContext *w, enum AVColorRange color_range)
 {
     const char *val = av_color_range_name(color_range);
     if (!val || color_range == AVCOL_RANGE_UNSPECIFIED) {
-        print_str_opt("color_range", fallback);
+        print_str_opt("color_range", "unknown");
     } else {
         print_str("color_range", val);
     }
@@ -2157,7 +2157,7 @@  static void show_frame(WriterContext *w, AVFrame *frame, AVStream *stream,
         print_int("top_field_first",        frame->top_field_first);
         print_int("repeat_pict",            frame->repeat_pict);
 
-        print_color_range(w, frame->color_range, "unknown");
+        print_color_range(w, frame->color_range);
         print_color_space(w, frame->colorspace);
         print_primaries(w, frame->color_primaries);
         print_color_trc(w, frame->color_trc);
@@ -2534,7 +2534,7 @@  static int show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_id
         else   print_str_opt("pix_fmt", "unknown");
         print_int("level",   par->level);
 
-        print_color_range(w, par->color_range, "N/A");
+        print_color_range(w, par->color_range);
         print_color_space(w, par->color_space);
         print_color_trc(w, par->color_trc);
         print_primaries(w, par->color_primaries);
diff --git a/tests/ref/fate/ffprobe_compact b/tests/ref/fate/ffprobe_compact
index 910837d..4a0f4ee 100644
--- a/tests/ref/fate/ffprobe_compact
+++ b/tests/ref/fate/ffprobe_compact
@@ -27,6 +27,6 @@  frame|media_type=video|stream_index=1|key_frame=1|pkt_pts=6144|pkt_pts_time=0.12
 packet|codec_type=video|stream_index=2|pts=6144|pts_time=0.120000|dts=6144|dts_time=0.120000|duration=2048|duration_time=0.040000|convergence_duration=N/A|convergence_duration_time=N/A|size=30000|pos=1023544|flags=K_
 frame|media_type=video|stream_index=2|key_frame=1|pkt_pts=6144|pkt_pts_time=0.120000|pkt_dts=6144|pkt_dts_time=0.120000|best_effort_timestamp=6144|best_effort_timestamp_time=0.120000|pkt_duration=2048|pkt_duration_time=0.040000|pkt_pos=1023544|pkt_size=30000|width=100|height=100|pix_fmt=rgb24|sample_aspect_ratio=1:1|pict_type=I|coded_picture_number=0|display_picture_number=0|interlaced_frame=0|top_field_first=0|repeat_pict=0|color_range=unknown|color_space=unknown|color_primaries=unknown|color_transfer=unknown|chroma_location=unspecified
 stream|index=0|codec_name=pcm_s16le|profile=unknown|codec_type=audio|codec_time_base=1/44100|codec_tag_string=PSD[16]|codec_tag=0x10445350|sample_fmt=s16|sample_rate=44100|channels=1|channel_layout=unknown|bits_per_sample=16|id=N/A|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/44100|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=705600|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=6|nb_read_packets=6|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|tag:E=mc²|tag:encoder=Lavc pcm_s16le
-stream|index=1|codec_name=rawvideo|profile=unknown|codec_type=video|codec_time_base=1/25|codec_tag_string=RGB[24]|codec_tag=0x18424752|width=320|height=240|coded_width=320|coded_height=240|has_b_frames=0|sample_aspect_ratio=1:1|display_aspect_ratio=4:3|pix_fmt=rgb24|level=-99|color_range=N/A|color_space=unknown|color_transfer=unknown|color_primaries=unknown|chroma_location=unspecified|field_order=unknown|timecode=N/A|refs=1|id=N/A|r_frame_rate=25/1|avg_frame_rate=25/1|time_base=1/51200|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=N/A|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=4|nb_read_packets=4|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|tag:title=foobar|tag:duration_ts=field-and-tag
 s-conflict-attempt|tag:encoder=Lavc rawvideo
-stream|index=2|codec_name=rawvideo|profile=unknown|codec_type=video|codec_time_base=1/25|codec_tag_string=RGB[24]|codec_tag=0x18424752|width=100|height=100|coded_width=100|coded_height=100|has_b_frames=0|sample_aspect_ratio=1:1|display_aspect_ratio=1:1|pix_fmt=rgb24|level=-99|color_range=N/A|color_space=unknown|color_transfer=unknown|color_primaries=unknown|chroma_location=unspecified|field_order=unknown|timecode=N/A|refs=1|id=N/A|r_frame_rate=25/1|avg_frame_rate=25/1|time_base=1/51200|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=N/A|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=4|nb_read_packets=4|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|tag:encoder=Lavc rawvideo
+stream|index=1|codec_name=rawvideo|profile=unknown|codec_type=video|codec_time_base=1/25|codec_tag_string=RGB[24]|codec_tag=0x18424752|width=320|height=240|coded_width=320|coded_height=240|has_b_frames=0|sample_aspect_ratio=1:1|display_aspect_ratio=4:3|pix_fmt=rgb24|level=-99|color_range=unknown|color_space=unknown|color_transfer=unknown|color_primaries=unknown|chroma_location=unspecified|field_order=unknown|timecode=N/A|refs=1|id=N/A|r_frame_rate=25/1|avg_frame_rate=25/1|time_base=1/51200|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=N/A|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=4|nb_read_packets=4|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|tag:title=foobar|tag:duration_ts=field-and
 -tags-conflict-attempt|tag:encoder=Lavc rawvideo
+stream|index=2|codec_name=rawvideo|profile=unknown|codec_type=video|codec_time_base=1/25|codec_tag_string=RGB[24]|codec_tag=0x18424752|width=100|height=100|coded_width=100|coded_height=100|has_b_frames=0|sample_aspect_ratio=1:1|display_aspect_ratio=1:1|pix_fmt=rgb24|level=-99|color_range=unknown|color_space=unknown|color_transfer=unknown|color_primaries=unknown|chroma_location=unspecified|field_order=unknown|timecode=N/A|refs=1|id=N/A|r_frame_rate=25/1|avg_frame_rate=25/1|time_base=1/51200|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=N/A|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=4|nb_read_packets=4|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|tag:encoder=Lavc rawvideo
 format|filename=tests/data/ffprobe-test.nut|nb_streams=3|nb_programs=0|format_name=nut|start_time=0.000000|duration=0.120000|size=1053624|bit_rate=70241600|probe_score=100|tag:title=ffprobe test file|tag:comment='A comment with CSV, XML & JSON special chars': <tag value="x">|tag:comment2=I ♥ Üñîçød€
diff --git a/tests/ref/fate/ffprobe_csv b/tests/ref/fate/ffprobe_csv
index 90e033f..dfbeb40 100644
--- a/tests/ref/fate/ffprobe_csv
+++ b/tests/ref/fate/ffprobe_csv
@@ -27,6 +27,6 @@  frame,video,1,1,6144,0.120000,6144,0.120000,6144,0.120000,2048,0.040000,793120,2
 packet,video,2,6144,0.120000,6144,0.120000,2048,0.040000,N/A,N/A,30000,1023544,K_
 frame,video,2,1,6144,0.120000,6144,0.120000,6144,0.120000,2048,0.040000,1023544,30000,100,100,rgb24,1:1,I,0,0,0,0,0,unknown,unknown,unknown,unknown,unspecified
 stream,0,pcm_s16le,unknown,audio,1/44100,PSD[16],0x10445350,s16,44100,1,unknown,16,N/A,0/0,0/0,1/44100,0,0.000000,N/A,N/A,705600,N/A,N/A,N/A,6,6,0,0,0,0,0,0,0,0,0,0,0,0,mc²,Lavc pcm_s16le
-stream,1,rawvideo,unknown,video,1/25,RGB[24],0x18424752,320,240,320,240,0,1:1,4:3,rgb24,-99,N/A,unknown,unknown,unknown,unspecified,unknown,N/A,1,N/A,25/1,25/1,1/51200,0,0.000000,N/A,N/A,N/A,N/A,N/A,N/A,4,4,0,0,0,0,0,0,0,0,0,0,0,0,foobar,field-and-tags-conflict-attempt,Lavc rawvideo
-stream,2,rawvideo,unknown,video,1/25,RGB[24],0x18424752,100,100,100,100,0,1:1,1:1,rgb24,-99,N/A,unknown,unknown,unknown,unspecified,unknown,N/A,1,N/A,25/1,25/1,1/51200,0,0.000000,N/A,N/A,N/A,N/A,N/A,N/A,4,4,0,0,0,0,0,0,0,0,0,0,0,0,Lavc rawvideo
+stream,1,rawvideo,unknown,video,1/25,RGB[24],0x18424752,320,240,320,240,0,1:1,4:3,rgb24,-99,unknown,unknown,unknown,unknown,unspecified,unknown,N/A,1,N/A,25/1,25/1,1/51200,0,0.000000,N/A,N/A,N/A,N/A,N/A,N/A,4,4,0,0,0,0,0,0,0,0,0,0,0,0,foobar,field-and-tags-conflict-attempt,Lavc rawvideo
+stream,2,rawvideo,unknown,video,1/25,RGB[24],0x18424752,100,100,100,100,0,1:1,1:1,rgb24,-99,unknown,unknown,unknown,unknown,unspecified,unknown,N/A,1,N/A,25/1,25/1,1/51200,0,0.000000,N/A,N/A,N/A,N/A,N/A,N/A,4,4,0,0,0,0,0,0,0,0,0,0,0,0,Lavc rawvideo
 format,tests/data/ffprobe-test.nut,3,0,nut,0.000000,0.120000,1053624,70241600,100,ffprobe test file,"'A comment with CSV, XML & JSON special chars': <tag value=""x"">",I ♥ Üñîçød€
diff --git a/tests/ref/fate/ffprobe_default b/tests/ref/fate/ffprobe_default
index bb3cf8a..7562867 100644
--- a/tests/ref/fate/ffprobe_default
+++ b/tests/ref/fate/ffprobe_default
@@ -621,7 +621,7 @@  sample_aspect_ratio=1:1
 display_aspect_ratio=4:3
 pix_fmt=rgb24
 level=-99
-color_range=N/A
+color_range=unknown
 color_space=unknown
 color_transfer=unknown
 color_primaries=unknown
@@ -676,7 +676,7 @@  sample_aspect_ratio=1:1
 display_aspect_ratio=1:1
 pix_fmt=rgb24
 level=-99
-color_range=N/A
+color_range=unknown
 color_space=unknown
 color_transfer=unknown
 color_primaries=unknown
diff --git a/tests/ref/fate/ffprobe_flat b/tests/ref/fate/ffprobe_flat
index ac657db..fc82ee9 100644
--- a/tests/ref/fate/ffprobe_flat
+++ b/tests/ref/fate/ffprobe_flat
@@ -562,7 +562,7 @@  streams.stream.1.sample_aspect_ratio="1:1"
 streams.stream.1.display_aspect_ratio="4:3"
 streams.stream.1.pix_fmt="rgb24"
 streams.stream.1.level=-99
-streams.stream.1.color_range="N/A"
+streams.stream.1.color_range="unknown"
 streams.stream.1.color_space="unknown"
 streams.stream.1.color_transfer="unknown"
 streams.stream.1.color_primaries="unknown"
@@ -615,7 +615,7 @@  streams.stream.2.sample_aspect_ratio="1:1"
 streams.stream.2.display_aspect_ratio="1:1"
 streams.stream.2.pix_fmt="rgb24"
 streams.stream.2.level=-99
-streams.stream.2.color_range="N/A"
+streams.stream.2.color_range="unknown"
 streams.stream.2.color_space="unknown"
 streams.stream.2.color_transfer="unknown"
 streams.stream.2.color_primaries="unknown"
diff --git a/tests/ref/fate/ffprobe_ini b/tests/ref/fate/ffprobe_ini
index 37e1563..a78690c 100644
--- a/tests/ref/fate/ffprobe_ini
+++ b/tests/ref/fate/ffprobe_ini
@@ -627,7 +627,7 @@  sample_aspect_ratio=1\:1
 display_aspect_ratio=4\:3
 pix_fmt=rgb24
 level=-99
-color_range=N/A
+color_range=unknown
 color_space=unknown
 color_transfer=unknown
 color_primaries=unknown
@@ -686,7 +686,7 @@  sample_aspect_ratio=1\:1
 display_aspect_ratio=1\:1
 pix_fmt=rgb24
 level=-99
-color_range=N/A
+color_range=unknown
 color_space=unknown
 color_transfer=unknown
 color_primaries=unknown
diff --git a/tests/ref/fate/mxf-probe-dnxhd b/tests/ref/fate/mxf-probe-dnxhd
index 6c7f4ad..c4de291 100644
--- a/tests/ref/fate/mxf-probe-dnxhd
+++ b/tests/ref/fate/mxf-probe-dnxhd
@@ -123,7 +123,7 @@  sample_aspect_ratio=1:1
 display_aspect_ratio=4:3
 pix_fmt=yuv422p
 level=-99
-color_range=N/A
+color_range=unknown
 color_space=bt709
 color_transfer=unknown
 color_primaries=unknown
diff --git a/tests/ref/fate/mxf-probe-dv25 b/tests/ref/fate/mxf-probe-dv25
index 0360691..6e02dd9 100644
--- a/tests/ref/fate/mxf-probe-dv25
+++ b/tests/ref/fate/mxf-probe-dv25
@@ -15,7 +15,7 @@  sample_aspect_ratio=16:15
 display_aspect_ratio=4:3
 pix_fmt=yuv420p
 level=-99
-color_range=N/A
+color_range=unknown
 color_space=unknown
 color_transfer=unknown
 color_primaries=unknown