[FFmpeg-devel] [PATCH] ffprobe: show bitmap subtitles size.

[Stefano Sabatini]

On date Tuesday 2013-09-03 23:11:42 +0200, Stefano Sabatini encoded:
> On date Tuesday 2013-09-03 16:43:30 +0200, Nicolas George encoded:
> > Fix trac ticket #2930.
> > 
> > Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> > ---
> >  ffprobe.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > 
> > I believe no update on the schema is necessary, since width and height are
> > already declared for video.
> > 
> 
> > I am a bit unsure about the "N/A" conformance to the schema, but the
> > declaration for packet.pos does not take it into account either.
> 
> I see this:
>     if (pkt->pos != -1) print_fmt    ("pos", "%"PRId64, pkt->pos);
>     else                print_str_opt("pos", "N/A");
> 
> but maybe I don't understand your concerns.

The point is that by using print_str_opt() it is not even shown in the
XML output, so there are no type declaration issues (indeed this was
one of the reason for introducing print_str_opt() in the first place).

> 
> > diff --git a/ffprobe.c b/ffprobe.c
> > index 23f8ec1..0d5fb95 100644
> > --- a/ffprobe.c
> > +++ b/ffprobe.c
> > @@ -1711,6 +1711,17 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
> >              print_int("channels",        dec_ctx->channels);
> >              print_int("bits_per_sample", av_get_bits_per_sample(dec_ctx->codec_id));
> >              break;
> > +
> > +        case AVMEDIA_TYPE_SUBTITLE:
> > +            if (dec_ctx->width)
> > +                print_int("width",       dec_ctx->width);
> > +            else
> 
> > +                print_str("width",       "N/A");
> 
> print_str_opt("width", "N/A");
> 
> same below.
> 
> This is required for fixed position field formats, you don't want to
> change the number of entries if a value is undefined. For other
> formats (e.g. JSON) ffprobe doesn't print the value at all (which is
> supposedly easier on the scripting side).
> 
> > +            if (dec_ctx->height)
> > +                print_int("height",      dec_ctx->height);
> > +            else
> > +                print_str("height",      "N/A");
> > +            break;
> >          }
> >      } else {
> >          print_str_opt("codec_type", "unknown");
> 
> LGTM otherwise and I had the very same idea, thanks.

I'll push the modified patch soon, thanks.
-- 
FFmpeg = Formidable Foolish Most Picky Exuberant Geisha