[FFmpeg-devel] [PATCH]Only test the first frame for malformed bitstreams

[Michael Niedermayer]

On Tue, Mar 26, 2013 at 07:47:47AM +0100, Reimar Döffinger wrote:
> On 25 Mar 2013, at 20:55, Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> > On Mon, Mar 25, 2013 at 12:39:17AM +0100, Michael Niedermayer wrote:
> >> On Mon, Mar 25, 2013 at 12:03:25AM +0100, Carl Eugen Hoyos wrote:
> >>> Hi!
> >>> 
> >>> Currently, the mpegts, the flv and the mov muxer test every h264 / aac frame 
> >>> for a missing bitstream filter, making remuxing of broken streams impossible.
> >>> 
> >>> Testing only the first frame should be sufficient to inform the user of a 
> >>> missing bitstream filter.
> >>> 
> >>> Attached patches fix tickets #1758 and #2380, I tested the remuxed files 
> >>> successfully with WMP and QT.
> >>> 
> >>> Please review, Carl Eugen
> >> 
> >>> mpegtsenc.c |    2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>> bdc3c59730f4da7cb36f7709b42a418574f4afeb  patch264ts.diff
> >>> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> >>> index 7016774..617b9a7 100644
> >>> --- a/libavformat/mpegtsenc.c
> >>> +++ b/libavformat/mpegtsenc.c
> >>> @@ -1093,7 +1093,7 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
> >>>         const uint8_t *p = buf, *buf_end = p+size;
> >>>         uint32_t state = -1;
> >>> 
> >>> -        if (pkt->size < 5 || AV_RB32(pkt->data) != 0x0000001) {
> >>> +        if (!st->codec->frame_number && (pkt->size < 5 || AV_RB32(pkt->data) != 0x0000001)) {
> >>>             av_log(s, AV_LOG_ERROR, "H.264 bitstream malformed, "
> >>>                    "no startcode found, use the h264_mp4toannexb bitstream filter (-bsf h264_mp4toannexb)\n");
> >>>             return AVERROR(EINVAL);
> >> 
> >> at least a warning should still be printed also
> >> st->nb_frames seems more correct than st->codec->frame_number if
> >> it works
> > 
> > This doesn't make much sense, should checking this condition only
> > on the first frame break the test for most streams?
> > I believe e.g. a TS e.g. from a BluRay should always still start
> > with a startcode, even if not bitstream filter is used.
> > At the same time, unless the bitstream filter throws away data
> > (in which case I think it is buggy), adding it will not
> > fix a stream not starting with a startcode.
> > Or in other words, I have the feeling this whole code is just
> > nonsense anyway, and adding more hacks on top of it won't make
> > it better.
> 
> Sorry, I confused parser and bitstream filter a bit.
> The point I am trying to make is that I believe testing only the first frame might still give the same false positives the patch is trying to fix, just more rarely.
> Wouldn't it make sense to change the test to check for the "bad" format, e.g. if the first few bytes seem to match the packet size for H.264?
> It will be less reliable in detecting a missing filter, but since this is mostly about helping against wrong usage that might not be such a big problem.

ideal would probably be to detect the wrong format and fix it
automatically without user interaction (but possibly with a warning)

until someone implements this, i think the patches are better than
no patches

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130326/6af26731/attachment.asc>