On 20.07.2017 16:19, Vittorio Giovara wrote:
On 20.07.2017 14:38, Vittorio Giovara wrote:
---
 ffprobe.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ffprobe.c b/ffprobe.c
index f6d9be0df9..412e2dadab 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -2105,6 +2105,12 @@ static void show_frame(WriterContext *w, AVFrame *frame, 
AVStream *stream,
         print_int("interlaced_frame",       frame->interlaced_frame);
         print_int("top_field_first",        frame->top_field_first);
         print_int("repeat_pict",            frame->repeat_pict);
+
+        print_str("color_range",     av_color_range_name(frame->color_range));
+        print_str("color_space",     av_color_space_name(frame->colorspace));
+        print_str("color_primaries", 
av_color_primaries_name(frame->color_primaries));
+        print_str("color_transfer",  av_color_transfer_name(frame->color_trc));
+        print_str("chroma_location", 
av_chroma_location_name(frame->chroma_location));

I guess this should look like

if (frame->... != ..._UNSPECIFIED)
    print_str(...);
else
    print_str_opt(...);

see the similar code lines handling color properties on stream level (~
line #2475).

Should it? That approach effectively hides these parameters from the
output if unknown, and I often want to know as much as possible when
hunting down parameters with read_frames (even that the filed is just
"unknown" and not missing). Also if these fields are always output, it
simplify parsing them quite a bit, don't you think so? I'd much rather
change the stream level code to output more information instead.


It depends on the writer whether print_str_opt actually writes to the output or not. For example when using CSV optional data is always written, when using XML it is skipped.

In my humble opinion for the XML output format it would only increase data size and make the parser slightly _more_ complex in case optional fields would be written (as the reader would have to check if the field value equals "unknown" or "N/A" strings to handle the value-not-valid situation). So I see no benefit in replacing print_str_opt with print_str globally.

         break;

     case AVMEDIA_TYPE_AUDIO:


The schema file at doc/ffprobe.xsd should be updated to reflect the new
fields.

Also I assume that some FATE references are changed by this patch?

Right, I'll update them in the next iteration, thanks for noticing.


Regards,
Tobias

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to