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

[Reimar Döffinger]

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.