On 11.09.2014 12:12, Stefano Sabatini wrote:
On date Thursday 2014-09-11 09:54:42 +0200, Tobias Rapp encoded:
Have now added documentation and cleaned up some unused variables
with the new patch. Also on a second thought the option names
"show_pixel_format_descriptions" or "show_pixel_formats" seem less
confusing to me than "show_pixel_descriptions" so I did choose the
shorter "show_pixel_formats".
What I would like to add in a follow-up patch is a way to output the
flags. I see different options:
a) Output a character per flag (R=RGB, A=ALPHA, B=BITSTREAM, ...)
but this needs extra documentation and might be confusing due to
collisions (P=PALETTE or P=PLANAR? B=BIG-ENDIAN or B=BITSTREAM?)
Example: flag="RAP"
b) Output a list of flag names separated by comma or space. This
would fit mostly with JSON (comma) or XML (space) output format
style, I guess.
Example 1: flag="rgb,alpha,palette"
Example 2: flag="rgb alpha palette"
c) Output each flag as a boolean value. This increases amount of
output but fits better to output formats like CSV or INI.
Example: is_rgb="1" has_alpha="1" is_bitstream="0" has_palette="1"
What do you think?
Flags are a collection, so probably it makes sense to print it as an
array. See how it is done with PRINT_DISPOSITION. That's not very
elegant though, but IMO the best solution for easing the following
parsing (no need to further serialize the flags value).
OK, this is like (c) with an own sub-section. Sounds fine.
From 5938c171472469e91bb2b7445100ab6509929ba7 Mon Sep 17 00:00:00 2001
From: Tobias Rapp <t.r...@noa-audio.com>
Date: Thu, 11 Sep 2014 09:16:52 +0200
Subject: [PATCH] ffprobe: add -show_pixel_formats option
Adds option -show_pixel_formats to ffprobe which lists all
available pixel formats with some details.
---
doc/ffprobe.texi | 6 +++++
doc/ffprobe.xsd | 15 +++++++++++
ffprobe.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 97 insertions(+), 2 deletions(-)
[...]
+static int get_bits_per_component(const AVPixFmtDescriptor *pixdesc)
+{
+ int c, bits, first_bits = 0;
+
+ for (c = 0; c < pixdesc->nb_components; c++) {
+ bits = pixdesc->comp[c].depth_minus1 + 1;
+ if (c == 0)
+ first_bits = bits;
+ else if (bits != first_bits)
+ return 0;
+ }
+
+ return first_bits;
+}
What if you compute this in the script? Since the bits_per_component
semantics is ambiguous it is probably better let the user compute it
herself.
So you suggest to output a component array like (in XML):
<pixel_format>
...
<component name="R" bit_depth="5"/>
<component name="G" bit_depth="6"/>
<component name="B" bit_depth="5"/>
</pixel_format>
instead?
+
+static const char *get_chroma_sub_sample_string(const AVPixFmtDescriptor
*pixdesc)
+{
+ if (!pixdesc)
+ return NULL;
+ if ((pixdesc->flags & AV_PIX_FMT_FLAG_RGB) || pixdesc->nb_components < 3)
+ return NULL;
+
+ if ((pixdesc->log2_chroma_w == 0) && (pixdesc->log2_chroma_h == 0))
+ return "4:4:4";
+ else if ((pixdesc->log2_chroma_w == 0) && (pixdesc->log2_chroma_h == 1))
+ return "4:4:0";
+ else if ((pixdesc->log2_chroma_w == 1) && (pixdesc->log2_chroma_h == 0))
+ return "4:2:2";
+ else if ((pixdesc->log2_chroma_w == 1) && (pixdesc->log2_chroma_h == 1))
+ return "4:2:0";
+ else if ((pixdesc->log2_chroma_w == 2) && (pixdesc->log2_chroma_h == 0))
+ return "4:1:1";
+ else if ((pixdesc->log2_chroma_w == 2) && (pixdesc->log2_chroma_h == 2))
+ return "4:1:0";
+ else
+ return "unknown";
+}
Why don't you just write the log2_chroma_w/h values?
That looks a bit low-level for me - like printing the audio channel
layout as "0x063f" instead of "7.1". From my POV I see ffprobe as a file
format reporting tool. For actually accessing pixel data I would use the
av* libraries.
Tobias
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
