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

[Clément Bœsch]

On Fri, Aug 01, 2014 at 01:54:57AM +0200, Carl Eugen Hoyos wrote:
> Hi!
> 
> 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.
> 
> Please review, Carl Eugen

> diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
> index 2adefc5..6e12ab2 100644
> --- a/ffmpeg_opt.c
> +++ b/ffmpeg_opt.c
> @@ -1869,8 +1869,16 @@ 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;
> +                    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.

> +                        (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 it is in the same spirit of what that
patch is trying to fix, so it should be included as part of it, or at
least the same patchset.

>                  }
>          }
>          /* do something with data? */

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140801/596ac367/attachment.asc>