[PATCH]Refuse to mux invalid transport streams
Hi! FFmpeg currently allows to mux transport streams that cannot be decoded, attached patch allows to report an error in such a case. Please comment, Carl Eugen
On Sun, May 19, 2013 at 10:21:59PM +0200, Carl Eugen Hoyos wrote:
Hi!
FFmpeg currently allows to mux transport streams that cannot be decoded, attached patch allows to report an error in such a case.
Please comment, Carl Eugen
mpegtsenc.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) f68bd75c40b81771225c4d347baaefba61ebbd5a patchmpegts.diff diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c index 0ddae65..ec40390 100644 --- a/libavformat/mpegtsenc.c +++ b/libavformat/mpegtsenc.c @@ -519,6 +519,43 @@ static int mpegts_write_header(AVFormatContext *s) int *pids; int ret;
+ for (i = 0; i < s->nb_streams; i++) { + st = s->streams[i]; + if (st->codec->codec_type == AVMEDIA_TYPE_DATA) + continue; + switch (st->codec->codec_id) { + case AV_CODEC_ID_CAVS: + case AV_CODEC_ID_DIRAC: + case AV_CODEC_ID_H264: + case AV_CODEC_ID_MPEG1VIDEO: + case AV_CODEC_ID_MPEG2VIDEO: + case AV_CODEC_ID_MPEG4: + case AV_CODEC_ID_AAC: + case AV_CODEC_ID_AAC_LATM: + case AV_CODEC_ID_AC3: + case AV_CODEC_ID_DTS: + case AV_CODEC_ID_EAC3: + case AV_CODEC_ID_MP2: + case AV_CODEC_ID_MP3: + case AV_CODEC_ID_S302M: + case AV_CODEC_ID_DVB_SUBTITLE: + continue; + } + switch (st->codec->codec_id) { + case AV_CODEC_ID_VC1: + case AV_CODEC_ID_TRUEHD: + case AV_CODEC_ID_PCM_BLURAY: + case AV_CODEC_ID_HDMV_PGS_SUBTITLE: + case AV_CODEC_ID_DVB_TELETEXT: + av_log(s, AV_LOG_ERROR, "Muxing %s in transport streams is not yet supported\n", + avcodec_get_name(st->codec->codec_id)); + return AVERROR_PATCHWELCOME; + } + av_log(s, AV_LOG_ERROR, "Codec %s not supported in transport streams\n", + avcodec_get_name(st->codec->codec_id)); + return AVERROR(EINVAL);
why dont you set AVOutputFormat.codec_tag / querry_codec ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I do not agree with what you have to say, but I'll defend to the death your right to say it. -- Voltaire
Michael Niedermayer <michaelni <at> gmx.at> writes:
+ av_log(s, AV_LOG_ERROR, "Codec %s not supported in transport streams\n", + avcodec_get_name(st->codec->codec_id)); + return AVERROR(EINVAL);
why dont you set AVOutputFormat.codec_tag
It is supposed to be set by the user.
/ querry_codec ?
I didn't know it and since it has no effect on ffmpeg it wouldn't help;-( Carl Eugen
On Mon, May 20, 2013 at 02:33:50PM +0000, Carl Eugen Hoyos wrote:
Michael Niedermayer <michaelni <at> gmx.at> writes:
+ av_log(s, AV_LOG_ERROR, "Codec %s not supported in transport streams\n", + avcodec_get_name(st->codec->codec_id)); + return AVERROR(EINVAL);
why dont you set AVOutputFormat.codec_tag
It is supposed to be set by the user.
no /** * List of supported codec_id-codec_tag pairs, ordered by "better * choice first". The arrays are all terminated by AV_CODEC_ID_NONE. */
/ querry_codec ?
I didn't know it and since it has no effect on ffmpeg it wouldn't help;-(
you could add a call to it into ffmpeg (if that doesnt break anything) or just call it from mpegtsenc.c [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are best at talking, realize last or never when they are wrong.
Michael Niedermayer <michaelni <at> gmx.at> writes:
+ av_log(s, AV_LOG_ERROR, "Codec %s not supported in transport streams\n", + avcodec_get_name(st->codec->codec_id)); + return AVERROR(EINVAL);
why dont you set AVOutputFormat.codec_tag
It is supposed to be set by the user.
(Sorry, I misunderstood). Are you sure setting a list of codec_tags is the right thing to do for the ts muxer? I always thought it works very different from how the tags are used by other muxers. [...]
/ querry_codec ?
I didn't know it and since it has no effect on ffmpeg it wouldn't help;-(
you could add a call to it into ffmpeg (if that doesnt break anything)
From a quick test, it breaks mkv muxing.
or just call it from mpegtsenc.c
Isn't that what my current approach does? (But without the indirection through a function with an apparently broken API.) Carl Eugen
Michael Niedermayer <michaelni <at> gmx.at> writes:
why dont you set AVOutputFormat.codec_tag / querry_codec ?
Both would break the nice difference between "does not work" and "is not implemented". Carl Eugen
Why two switches on the same? Just merge them. -- Andrey Utkin
Hi! On Sunday 19 May 2013 10:21:59 pm Carl Eugen Hoyos wrote:
FFmpeg currently allows to mux transport streams that cannot be decoded, attached patch allows to report an error in such a case.
I still believe this is an important issue (that hits users regularly). I also think it is important to distinguish between codecs not supported in transport streams and codecs that are supported but support is not implemented in FFmpeg. Please comment, Carl Eugen
On Sun, Jun 01, 2014 at 02:58:41PM +0200, Carl Eugen Hoyos wrote:
Hi!
On Sunday 19 May 2013 10:21:59 pm Carl Eugen Hoyos wrote:
FFmpeg currently allows to mux transport streams that cannot be decoded, attached patch allows to report an error in such a case.
I still believe this is an important issue (that hits users regularly). I also think it is important to distinguish between codecs not supported in transport streams and codecs that are supported but support is not implemented in FFmpeg.
Please comment, Carl Eugen
mpegtsenc.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) e1e90eb9e99ccd0c74f024ce9ae6c4ff8dcf9234 patchmpegts.diff diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c index f773019..9571789 100644 --- a/libavformat/mpegtsenc.c +++ b/libavformat/mpegtsenc.c @@ -608,6 +608,44 @@ static int mpegts_write_header(AVFormatContext *s) int *pids; int ret;
+ for (i = 0; i < s->nb_streams; i++) { + st = s->streams[i]; + if (st->codec->codec_type == AVMEDIA_TYPE_DATA || + st->codec->strict_std_compliance <= FF_COMPLIANCE_EXPERIMENTAL) + continue; + switch (st->codec->codec_id) { + case AV_CODEC_ID_CAVS: + case AV_CODEC_ID_DIRAC: + case AV_CODEC_ID_H264: + case AV_CODEC_ID_MPEG1VIDEO: + case AV_CODEC_ID_MPEG2VIDEO: + case AV_CODEC_ID_MPEG4: + case AV_CODEC_ID_AAC: + case AV_CODEC_ID_AAC_LATM: + case AV_CODEC_ID_AC3: + case AV_CODEC_ID_DTS: + case AV_CODEC_ID_EAC3: + case AV_CODEC_ID_MP2: + case AV_CODEC_ID_MP3: + case AV_CODEC_ID_S302M: + case AV_CODEC_ID_DVB_SUBTITLE: + case AV_CODEC_ID_DVB_TELETEXT: + continue; + } + switch (st->codec->codec_id) { + case AV_CODEC_ID_VC1: + case AV_CODEC_ID_TRUEHD: + case AV_CODEC_ID_PCM_BLURAY: + case AV_CODEC_ID_HDMV_PGS_SUBTITLE: + av_log(s, AV_LOG_ERROR, "Muxing %s in transport streams is not yet implemented, use -strict experimental to write invalid files\n", + avcodec_get_name(st->codec->codec_id)); + return AVERROR_PATCHWELCOME; + } + av_log(s, AV_LOG_ERROR, "Codec %s not supported in transport streams, use -strict experimental to write invalid files\n", + avcodec_get_name(st->codec->codec_id)); + return AVERROR(EINVAL); + }
can this be in AVOutputFormat.query_codec() ? because it would fit better there even if its called from mpegts_write_header() instead of a generic place [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship naturally arises out of democracy, and the most aggravated form of tyranny and slavery out of the most extreme liberty. -- Plato
participants (3)
-
Andrey Utkin -
Carl Eugen Hoyos -
Michael Niedermayer