diff mbox series

[FFmpeg-devel] lavc: add detection of forced subtitles

Message ID 20220919175534.102124-1-scott.the.elm@gmail.com
State New
Headers show
Series [FFmpeg-devel] lavc: add detection of forced subtitles | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Scott Theisen Sept. 19, 2022, 5:55 p.m. UTC
This is from the following MythTV commits:
libavcodec: add support for detecting forced subtitle segments. https://github.com/MythTV/mythtv/commit/5099f1a5777966fba482b623e581c1eef5c8fc09
This adds forced subtitle segment detection to the PGS subtitle decoder and
copies the result to AVSubtitle.

Subtitles: Flag forced DVD subtitles. https://github.com/MythTV/mythtv/commit/78f71eecdbd53ba92af2ad639b32564c01f24563
---
 libavcodec/avcodec.h   | 1 +
 libavcodec/dvdsubdec.c | 4 +++-
 libavcodec/pgssubdec.c | 4 ++++
 3 files changed, 8 insertions(+), 1 deletion(-)

Comments

Andreas Rheinhardt Sept. 19, 2022, 8:53 p.m. UTC | #1
Scott Theisen:
> This is from the following MythTV commits:
> libavcodec: add support for detecting forced subtitle segments. https://github.com/MythTV/mythtv/commit/5099f1a5777966fba482b623e581c1eef5c8fc09
> This adds forced subtitle segment detection to the PGS subtitle decoder and
> copies the result to AVSubtitle.
> 
> Subtitles: Flag forced DVD subtitles. https://github.com/MythTV/mythtv/commit/78f71eecdbd53ba92af2ad639b32564c01f24563
> ---
>  libavcodec/avcodec.h   | 1 +
>  libavcodec/dvdsubdec.c | 4 +++-
>  libavcodec/pgssubdec.c | 4 ++++
>  3 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 7db5d1b1c5..b9aa9efb2f 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2331,6 +2331,7 @@ typedef struct AVSubtitle {
>      unsigned num_rects;
>      AVSubtitleRect **rects;
>      int64_t pts;    ///< Same as packet pts, in AV_TIME_BASE
> +    int forced;

sizeof(AVSubtitle) is part of the public ABI, no additions to it are
possible. I wanted to suggest using AVSubtitleRect.flags, but apparently
AV_SUBTITLE_FLAG_FORCED already exists. And it seems that both the
dvdsub as well as the pgssub decoders already set this.

>  } AVSubtitle;
>  
>  /**
> diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
> index a5da0d7b08..1504706d52 100644
> --- a/libavcodec/dvdsubdec.c
> +++ b/libavcodec/dvdsubdec.c
> @@ -418,8 +418,10 @@ static int decode_dvd_subtitles(DVDSubContext *ctx, AVSubtitle *sub_header,
>              break;
>          cmd_pos = next_cmd_pos;
>      }
> -    if (sub_header->num_rects > 0)
> +    if (sub_header->num_rects > 0) {
> +        sub_header->forced = is_menu;
>          return is_menu;
> +    }
>   fail:
>      reset_rects(sub_header);
>      return -1;
> diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c
> index 5f76f12615..7c953263bb 100644
> --- a/libavcodec/pgssubdec.c
> +++ b/libavcodec/pgssubdec.c
> @@ -62,6 +62,7 @@ typedef struct PGSSubPresentation {
>      int id_number;
>      int palette_id;
>      int object_count;
> +    int object_forced;
>      PGSSubObjectRef objects[MAX_OBJECT_REFS];
>      int64_t pts;
>  } PGSSubPresentation;
> @@ -466,6 +467,8 @@ static int parse_presentation_segment(AVCodecContext *avctx,
>              object->crop_h = bytestream_get_be16(&buf);
>          }
>  
> +        ctx->presentation.object_forced = (ctx->presentation.objects[i].composition_flag & 0x40) >> 6;
> +
>          ff_dlog(avctx, "Subtitle Placement x=%d, y=%d\n",
>                  object->x, object->y);
>  
> @@ -505,6 +508,7 @@ static int display_end_segment(AVCodecContext *avctx, AVSubtitle *sub,
>      memset(sub, 0, sizeof(*sub));
>      sub->pts = pts;
>      ctx->presentation.pts = AV_NOPTS_VALUE;
> +    sub->forced = ctx->presentation.object_forced;
>      sub->start_display_time = 0;
>      // There is no explicit end time for PGS subtitles.  The end time
>      // is defined by the start of the next sub which may contain no
Hendrik Leppkes Sept. 19, 2022, 8:58 p.m. UTC | #2
On Mon, Sep 19, 2022 at 10:53 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Scott Theisen:
> > This is from the following MythTV commits:
> > libavcodec: add support for detecting forced subtitle segments. https://github.com/MythTV/mythtv/commit/5099f1a5777966fba482b623e581c1eef5c8fc09
> > This adds forced subtitle segment detection to the PGS subtitle decoder and
> > copies the result to AVSubtitle.
> >
> > Subtitles: Flag forced DVD subtitles. https://github.com/MythTV/mythtv/commit/78f71eecdbd53ba92af2ad639b32564c01f24563
> > ---
> >  libavcodec/avcodec.h   | 1 +
> >  libavcodec/dvdsubdec.c | 4 +++-
> >  libavcodec/pgssubdec.c | 4 ++++
> >  3 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 7db5d1b1c5..b9aa9efb2f 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -2331,6 +2331,7 @@ typedef struct AVSubtitle {
> >      unsigned num_rects;
> >      AVSubtitleRect **rects;
> >      int64_t pts;    ///< Same as packet pts, in AV_TIME_BASE
> > +    int forced;
>
> sizeof(AVSubtitle) is part of the public ABI, no additions to it are
> possible. I wanted to suggest using AVSubtitleRect.flags, but apparently
> AV_SUBTITLE_FLAG_FORCED already exists. And it seems that both the
> dvdsub as well as the pgssub decoders already set this.
>

Which is also more correct, since in PGS for example you can have both
active rects that are forced, and those that are not, at the same
time.

- Hendrik
Scott Theisen Sept. 21, 2022, 12:21 a.m. UTC | #3
On 9/19/22 16:58, Hendrik Leppkes wrote:
> On Mon, Sep 19, 2022 at 10:53 PM Andreas Rheinhardt
> <andreas.rheinhardt@outlook.com> wrote:
>> Scott Theisen:
>>> This is from the following MythTV commits:
>>> libavcodec: add support for detecting forced subtitle segments. https://github.com/MythTV/mythtv/commit/5099f1a5777966fba482b623e581c1eef5c8fc09
>>> This adds forced subtitle segment detection to the PGS subtitle decoder and
>>> copies the result to AVSubtitle.
>>>
>>> Subtitles: Flag forced DVD subtitles. https://github.com/MythTV/mythtv/commit/78f71eecdbd53ba92af2ad639b32564c01f24563
>>> ---
>>>   libavcodec/avcodec.h   | 1 +
>>>   libavcodec/dvdsubdec.c | 4 +++-
>>>   libavcodec/pgssubdec.c | 4 ++++
>>>   3 files changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>> index 7db5d1b1c5..b9aa9efb2f 100644
>>> --- a/libavcodec/avcodec.h
>>> +++ b/libavcodec/avcodec.h
>>> @@ -2331,6 +2331,7 @@ typedef struct AVSubtitle {
>>>       unsigned num_rects;
>>>       AVSubtitleRect **rects;
>>>       int64_t pts;    ///< Same as packet pts, in AV_TIME_BASE
>>> +    int forced;
>> sizeof(AVSubtitle) is part of the public ABI, no additions to it are
>> possible. I wanted to suggest using AVSubtitleRect.flags, but apparently
>> AV_SUBTITLE_FLAG_FORCED already exists. And it seems that both the
>> dvdsub as well as the pgssub decoders already set this.
>>
> Which is also more correct, since in PGS for example you can have both
> active rects that are forced, and those that are not, at the same
> time.
>
> - Hendrik

Thanks for the suggestion, I'll look into switching MythTV to that. 
MythTV's handling of subtitles involves a few customizations to FFmpeg 
and I thought this would be acceptable.  Apparently not, and there 
already exists a method for this.

Thanks,

Scott
diff mbox series

Patch

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 7db5d1b1c5..b9aa9efb2f 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -2331,6 +2331,7 @@  typedef struct AVSubtitle {
     unsigned num_rects;
     AVSubtitleRect **rects;
     int64_t pts;    ///< Same as packet pts, in AV_TIME_BASE
+    int forced;
 } AVSubtitle;
 
 /**
diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
index a5da0d7b08..1504706d52 100644
--- a/libavcodec/dvdsubdec.c
+++ b/libavcodec/dvdsubdec.c
@@ -418,8 +418,10 @@  static int decode_dvd_subtitles(DVDSubContext *ctx, AVSubtitle *sub_header,
             break;
         cmd_pos = next_cmd_pos;
     }
-    if (sub_header->num_rects > 0)
+    if (sub_header->num_rects > 0) {
+        sub_header->forced = is_menu;
         return is_menu;
+    }
  fail:
     reset_rects(sub_header);
     return -1;
diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c
index 5f76f12615..7c953263bb 100644
--- a/libavcodec/pgssubdec.c
+++ b/libavcodec/pgssubdec.c
@@ -62,6 +62,7 @@  typedef struct PGSSubPresentation {
     int id_number;
     int palette_id;
     int object_count;
+    int object_forced;
     PGSSubObjectRef objects[MAX_OBJECT_REFS];
     int64_t pts;
 } PGSSubPresentation;
@@ -466,6 +467,8 @@  static int parse_presentation_segment(AVCodecContext *avctx,
             object->crop_h = bytestream_get_be16(&buf);
         }
 
+        ctx->presentation.object_forced = (ctx->presentation.objects[i].composition_flag & 0x40) >> 6;
+
         ff_dlog(avctx, "Subtitle Placement x=%d, y=%d\n",
                 object->x, object->y);
 
@@ -505,6 +508,7 @@  static int display_end_segment(AVCodecContext *avctx, AVSubtitle *sub,
     memset(sub, 0, sizeof(*sub));
     sub->pts = pts;
     ctx->presentation.pts = AV_NOPTS_VALUE;
+    sub->forced = ctx->presentation.object_forced;
     sub->start_display_time = 0;
     // There is no explicit end time for PGS subtitles.  The end time
     // is defined by the start of the next sub which may contain no