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

[Carl Eugen Hoyos]

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
-------------- next part --------------
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;
+                    }
                 }
         }
         /* do something with data? */