[FFmpeg-devel] [PATCH v1] fftools/ffmpeg: set AV_PKT_FLAG_KEY for the subtitle packet

[Limin Wang]

On Fri, Mar 27, 2020 at 01:47:10AM +0100, Michael Niedermayer wrote:
> On Thu, Mar 26, 2020 at 10:47:37AM +0100, Anton Khirnov wrote:
> > Quoting Andreas Rheinhardt (2020-03-20 17:51:41)
> > > Jan Ekström:
> > > > On Fri, Mar 20, 2020 at 5:45 PM <lance.lmwang at gmail.com> wrote:
> > > >>
> > > >> From: Limin Wang <lance.lmwang at gmail.com>
> > > >>
> > > >> This fixes webvtt segment output.
> > > >>
> > > >> Please testing with the following command and check the output:
> > > >> ./ffmpeg -i ../fate-suite/sub/MicroDVD_capability_tester.srt -f segment -segment_time 10 \
> > > >>     -segment_list_size 0  -segment_list sub.m3u8 -segment_format webvtt -scodec webvtt sub-%d.vtt
> > > >>
> > > >>
> > > >> Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> > > >> ---
> > > >>  fftools/ffmpeg.c                 |  1 +
> > > >>  tests/ref/fate/binsub-movtextenc |  2 +-
> > > >>  tests/ref/fate/sub2video         | 86 ++++++++++++++++----------------
> > > >>  3 files changed, 45 insertions(+), 44 deletions(-)
> > > >>
> > > >> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > > >> index aaaf241314..c5a2d0731d 100644
> > > >> --- a/fftools/ffmpeg.c
> > > >> +++ b/fftools/ffmpeg.c
> > > >> @@ -1054,6 +1054,7 @@ static void do_subtitle_out(OutputFile *of,
> > > >>              else
> > > >>                  pkt.pts += av_rescale_q(sub->end_display_time, (AVRational){ 1, 1000 }, ost->mux_timebase);
> > > >>          }
> > > >> +        pkt.flags |= AV_PKT_FLAG_KEY;
> > > >>          pkt.dts = pkt.pts;
> > > >>          output_packet(of, &pkt, ost, 0);
> > > >>      }
> > > > 
> > > > I do wonder if this is just a case of people forgetting to set the
> > > > flag for the relevant packets in the relevant modules?
> > > > 
> > > > I'm not sure if all API users should be forced to handle this
> > > > separately. If the packets are decode'able by themselves, they should
> > > > be marked as such.
> > > > 
> > > > (Unfortunately, this probably means that all subtitle encoders and
> > > > text-based subtitle format demuxers would have to be updated where
> > > > this flag is not set)
> > > > 
> > > av_read_frame() already sets the AV_PKT_FLAG_KEY-flag for all subtitle
> > > packets (see is_intra_only() in libavformat/utils.c; the subtitle
> > > demuxer based around FFDemuxSubtitlesQueues actually have the flag set
> > > even before it reaches is_intra_only()). One could do something similar
> > > in libavformat/mux.c.
> > > But are we actually sure that all subtitle packets are decodable by
> > > themselves? IIRC this is not true for all PGS subtitles.
> > > 
> > > - Andreas
> > > 
> > > PS: The semantics of the AV_CODEC_PROP_INTRA_ONLY-flag seem to be based
> > > around the assumption that subtitle packets are always intra-only: It is
> > > only for video and audio-codecs only.
> > 
> > I would interpret it as simply not being defined for subtitles, rather
> > than saying they are all always intra only. So we can always add new
> > semantics for subtitles to it.
> 
> +1
I have updated the patch, please review whether it's expected.

> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> The real ebay dictionary, page 2
> "100% positive feedback" - "All either got their money back or didnt complain"
> "Best seller ever, very honest" - "Seller refunded buyer after failed scam"



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


-- 
Thanks,
Limin Wang