[FFmpeg-devel] [PATCH]Do not select a default subtitle output stream with incorrect subtitle type

Michael Niedermayer michaelni at gmx.at
Thu Aug 7 12:48:36 CEST 2014


On Tue, Aug 05, 2014 at 09:59:13PM +0200, Carl Eugen Hoyos wrote:
> On Friday 01 August 2014 10:04:17 am Clément Bœsch wrote:
> > On Fri, Aug 01, 2014 at 01:54:57AM +0200, Carl Eugen Hoyos wrote:
> > > When transcoding an input file with subtitles to mkv, ffmpeg by default
> > > tries to encode the subtitles even if the input contains bitmap
> > > subtitles. Attached patch should fix this issue reported in ticket #3819
> > > (and before iirc).
> 
> > > This patch currently does not work correctly for teletext input streams
> > > because no properties are defined for teletext, I suspect it should be
> > > (AV_CODEC_PROP_TEXT_SUB | AV_CODEC_PROP_BITMAP_SUB). In any case, this
> > > corner-case should not affect this patch imo.
> 
> I worked around this by automatically mapping subtitles if no properties 
> are set.
> 
> > > +                    if (subtitle_codec_name ||
> > > +                       
> > > avcodec_find_encoder(oc->oformat->subtitle_codec) && +                   
> > >    
> > > avcodec_descriptor_get(avcodec_find_encoder(oc->oformat->subtitle_codec)-
> > >>id) && +                       
> > > avcodec_descriptor_get(input_streams[i]->st->codec->codec_id) && +       
> > >                
> > > avcodec_descriptor_get(avcodec_find_encoder(oc->oformat->subtitle_codec)-
> > >>id)->props & +                       
> > > avcodec_descriptor_get(input_streams[i]->st->codec->codec_id)->props &
> >
> > You have a scope opened here, please use 2 intermediate const to store the
> > 2 descriptor pointers, because it's currently unreadable.
> 
> Slightly more readable version attached.
> 
> >
> > > +                        (AV_CODEC_PROP_TEXT_SUB |
> > > AV_CODEC_PROP_BITMAP_SUB)) { +                       
> > > new_subtitle_stream(o, oc, i);
> > > +                        break;
> > > +                    }
> >
> > BTW, do we have an obvious warning nowadays if someone tries to map a
> > bitmap sub to a text one?
> 
> I believe the warning is the reason for ticket #3819.
> In any case, this is orthoganal to this patch imo: The patch tries 
> to cover automatic subtitle mapping (in the useless case), iiuc 
> you (also) mean the case when a user specifies a subtitle encoder 
> that will not work.
> 
> Thank you, Carl Eugen

>  ffmpeg_opt.c |   23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 977bbbc7d31d57d206f9fdfb9d73c35388f97a84  patchdefaultsub2.diff
> diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
> index 9604a6a..1962cf4 100644
> --- a/ffmpeg_opt.c
> +++ b/ffmpeg_opt.c
> @@ -1870,8 +1870,27 @@ static int open_output_file(OptionsContext *o, const char *filename)
>          if (!o->subtitle_disable && (avcodec_find_encoder(oc->oformat->subtitle_codec) || subtitle_codec_name)) {
>              for (i = 0; i < nb_input_streams; i++)
>                  if (input_streams[i]->st->codec->codec_type == AVMEDIA_TYPE_SUBTITLE) {
> -                    new_subtitle_stream(o, oc, i);
> -                    break;
> +                    AVCodecDescriptor const *input_descriptor =
> +                        avcodec_descriptor_get(input_streams[i]->st->codec->codec_id);
> +                    AVCodecDescriptor const *output_descriptor = NULL;
> +                    AVCodec const *output_codec =
> +                        avcodec_find_encoder(oc->oformat->subtitle_codec);
> +                    int input_props = 0, output_props = 0;
> +                    if (output_codec)
> +                        output_descriptor = avcodec_descriptor_get(output_codec->id);
> +                    if (input_descriptor)
> +                        input_props = input_descriptor->props & (AV_CODEC_PROP_TEXT_SUB | AV_CODEC_PROP_BITMAP_SUB);
> +                    if (output_descriptor)
> +                        output_props = output_descriptor->props & (AV_CODEC_PROP_TEXT_SUB | AV_CODEC_PROP_BITMAP_SUB);
> +                    if (subtitle_codec_name ||
> +                        input_props & output_props ||
> +                        // Map dvb teletext which has neither property to any output subtitle encoder
> +                        input_descriptor && output_descriptor &&
> +                        (!input_descriptor->props ||
> +                         !output_descriptor->props)) {
> +                        new_subtitle_stream(o, oc, i);
> +                        break;
> +                    }

should be ok but why does dvb teletext not have a TEXT/BITMAP prop ?


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 3
"Rare item" - "Common item with rare defect or maybe just a lie"
"Professional" - "'Toy' made in china, not functional except as doorstop"
"Experts will know" - "The seller hopes you are not an expert"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140807/512353f1/attachment.asc>


More information about the ffmpeg-devel mailing list