diff mbox series

[FFmpeg-devel,v3,07/18] fftools/play, probe: Adjust for subtitle format type change

Message ID MN2PR04MB5981AD4E71107EE031DBF9A1BAD79@MN2PR04MB5981.namprd04.prod.outlook.com
State Superseded, archived
Headers show
Series None | expand

Commit Message

Soft Works Sept. 11, 2021, 6:02 a.m. UTC
Signed-off-by: softworkz <softworkz@hotmail.com>
---
 fftools/ffplay.c  |  2 +-
 fftools/ffprobe.c | 23 ++++++++++++++++++++++-
 2 files changed, 23 insertions(+), 2 deletions(-)

Comments

Tobias Rapp Sept. 13, 2021, 7:41 a.m. UTC | #1
On 11.09.2021 08:02, Soft Works wrote:
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>   fftools/ffplay.c  |  2 +-
>   fftools/ffprobe.c | 23 ++++++++++++++++++++++-
>   2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index 46758b9f55..f6a4d242c3 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -2250,7 +2250,7 @@ static int subtitle_thread(void *arg)
>   
>           pts = 0;
>   
> -        if (got_subtitle && sp->sub.format == 0) {
> +        if (got_subtitle && sp->sub.format == AV_SUBTITLE_FMT_BITMAP) {
>               if (sp->sub.pts != AV_NOPTS_VALUE)
>                   pts = sp->sub.pts / (double)AV_TIME_BASE;
>               sp->pts = pts;
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index acfec09656..fb55f3b292 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -2212,6 +2212,7 @@ static void show_subtitle(WriterContext *w, AVSubtitle *sub, AVStream *stream,
>                             AVFormatContext *fmt_ctx)
>   {
>       AVBPrint pbuf;
> +    const char *s;
>   
>       av_bprint_init(&pbuf, 1, AV_BPRINT_SIZE_UNLIMITED);
>   
> @@ -2220,7 +2221,27 @@ static void show_subtitle(WriterContext *w, AVSubtitle *sub, AVStream *stream,
>       print_str ("media_type",         "subtitle");
>       print_ts  ("pts",                 sub->pts);
>       print_time("pts_time",            sub->pts, &AV_TIME_BASE_Q);
> -    print_int ("format",              sub->format);
> +
> +    // Remain compatible with previous outputs?
> +    switch (sub->format) {
> +    case AV_SUBTITLE_FMT_BITMAP:
> +        print_int ("format",         0);
> +        break;
> +    case AV_SUBTITLE_FMT_TEXT:
> +        print_int ("format",         1);
> +        break;
> +    case AV_SUBTITLE_FMT_ASS:
> +        print_int ("format",         1);
> +        break;
> +    default:
> +        print_int ("format",         -1);
> +        break;
> +    }
> +
> +    s = av_get_subtitle_fmt_name(sub->format);
> +    if (s) print_str    ("format_str", s);
> +    else   print_str_opt("format_str", "unknown");
> +
>       print_int ("start_display_time",  sub->start_display_time);
>       print_int ("end_display_time",    sub->end_display_time);
>       print_int ("num_rects",           sub->num_rects);
> 

In any case the new attribute should be added to the "subtitle" element 
type definition in doc/ffprobe.xsd.

Regards,
Tobias
Soft Works Sept. 13, 2021, 8:07 a.m. UTC | #2
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Tobias Rapp
> Sent: Monday, 13 September 2021 09:41
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v3 07/18] fftools/play, probe:
> Adjust for subtitle format type change
> 
> On 11.09.2021 08:02, Soft Works wrote:
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> >   fftools/ffplay.c  |  2 +-
> >   fftools/ffprobe.c | 23 ++++++++++++++++++++++-
> >   2 files changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> > index 46758b9f55..f6a4d242c3 100644
> > --- a/fftools/ffplay.c
> > +++ b/fftools/ffplay.c
> > @@ -2250,7 +2250,7 @@ static int subtitle_thread(void *arg)
> >
> >           pts = 0;
> >
> > -        if (got_subtitle && sp->sub.format == 0) {
> > +        if (got_subtitle && sp->sub.format ==
> AV_SUBTITLE_FMT_BITMAP) {
> >               if (sp->sub.pts != AV_NOPTS_VALUE)
> >                   pts = sp->sub.pts / (double)AV_TIME_BASE;
> >               sp->pts = pts;
> > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> > index acfec09656..fb55f3b292 100644
> > --- a/fftools/ffprobe.c
> > +++ b/fftools/ffprobe.c
> > @@ -2212,6 +2212,7 @@ static void show_subtitle(WriterContext *w,
> AVSubtitle *sub, AVStream *stream,
> >                             AVFormatContext *fmt_ctx)
> >   {
> >       AVBPrint pbuf;
> > +    const char *s;
> >
> >       av_bprint_init(&pbuf, 1, AV_BPRINT_SIZE_UNLIMITED);
> >
> > @@ -2220,7 +2221,27 @@ static void show_subtitle(WriterContext *w,
> AVSubtitle *sub, AVStream *stream,
> >       print_str ("media_type",         "subtitle");
> >       print_ts  ("pts",                 sub->pts);
> >       print_time("pts_time",            sub->pts, &AV_TIME_BASE_Q);
> > -    print_int ("format",              sub->format);
> > +
> > +    // Remain compatible with previous outputs?
> > +    switch (sub->format) {
> > +    case AV_SUBTITLE_FMT_BITMAP:
> > +        print_int ("format",         0);
> > +        break;
> > +    case AV_SUBTITLE_FMT_TEXT:
> > +        print_int ("format",         1);
> > +        break;
> > +    case AV_SUBTITLE_FMT_ASS:
> > +        print_int ("format",         1);
> > +        break;
> > +    default:
> > +        print_int ("format",         -1);
> > +        break;
> > +    }
> > +
> > +    s = av_get_subtitle_fmt_name(sub->format);
> > +    if (s) print_str    ("format_str", s);
> > +    else   print_str_opt("format_str", "unknown");
> > +
> >       print_int ("start_display_time",  sub->start_display_time);
> >       print_int ("end_display_time",    sub->end_display_time);
> >       print_int ("num_rects",           sub->num_rects);
> >
> 
> In any case the new attribute should be added to the "subtitle"
> element
> type definition in doc/ffprobe.xsd.

Yea, that's the easy side. The question is for what to do about the 
'format' attribute values - whether to keep the numbers compatible
(like the patch currently does), change the meaning of the numbers
being output or add an additional attribute with the "new" numbers.

softworkz
diff mbox series

Patch

diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index 46758b9f55..f6a4d242c3 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -2250,7 +2250,7 @@  static int subtitle_thread(void *arg)
 
         pts = 0;
 
-        if (got_subtitle && sp->sub.format == 0) {
+        if (got_subtitle && sp->sub.format == AV_SUBTITLE_FMT_BITMAP) {
             if (sp->sub.pts != AV_NOPTS_VALUE)
                 pts = sp->sub.pts / (double)AV_TIME_BASE;
             sp->pts = pts;
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index acfec09656..fb55f3b292 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -2212,6 +2212,7 @@  static void show_subtitle(WriterContext *w, AVSubtitle *sub, AVStream *stream,
                           AVFormatContext *fmt_ctx)
 {
     AVBPrint pbuf;
+    const char *s;
 
     av_bprint_init(&pbuf, 1, AV_BPRINT_SIZE_UNLIMITED);
 
@@ -2220,7 +2221,27 @@  static void show_subtitle(WriterContext *w, AVSubtitle *sub, AVStream *stream,
     print_str ("media_type",         "subtitle");
     print_ts  ("pts",                 sub->pts);
     print_time("pts_time",            sub->pts, &AV_TIME_BASE_Q);
-    print_int ("format",              sub->format);
+
+    // Remain compatible with previous outputs?
+    switch (sub->format) {
+    case AV_SUBTITLE_FMT_BITMAP:
+        print_int ("format",         0);
+        break;
+    case AV_SUBTITLE_FMT_TEXT:
+        print_int ("format",         1);
+        break;
+    case AV_SUBTITLE_FMT_ASS:
+        print_int ("format",         1);
+        break;
+    default:
+        print_int ("format",         -1);
+        break;
+    }
+
+    s = av_get_subtitle_fmt_name(sub->format);
+    if (s) print_str    ("format_str", s);
+    else   print_str_opt("format_str", "unknown");
+
     print_int ("start_display_time",  sub->start_display_time);
     print_int ("end_display_time",    sub->end_display_time);
     print_int ("num_rects",           sub->num_rects);