[FFmpeg-devel] [PATCH]: Fix decoding of mpegts streams with h264 video that does *NOT* have b frames

[Tony Strauss]

I'm cross-posting to ffmpeg-devel and libav-devel, as this issue
exists in both codebases.  I've attached the patch and reproduced
below.  I don't have a lot of experience with the ffmpeg code base.

It originally was reported in:
https://roundup.libav.org/issue2222

The issue was created by the patch accepted in this e-mail thread:
https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2010-May/096245.html

I ran across this issue because pts and dts were getting nuked by this
block from utils.c for some of my mpegts files:
    if(delay==1 && pkt->dts == pkt->pts && pkt->dts != AV_NOPTS_VALUE
&& presentation_delayed){
        av_log(s, AV_LOG_DEBUG, "invalid dts/pts combination\n");
        pkt->dts= pkt->pts= AV_NOPTS_VALUE;
    }

You can reproduce by running:
ffprobe -loglevel debug -show_packets 720.no_b_frames.ts

The test file may be downloaded from
https://s3.amazonaws.com/tony-strauss-public/720.no_b_frames.ts.  The
key thing is that this file was encoded without b frames, so pts ==
dts for every frame.  has_b_frames, however, was getting incorrectly
set by ffmpeg's h264 parser, which caused delay to be true and that
block to be executed.

Basically, the earlier patch made h264_parse() invoke
ff_h264_decode_extradata() the first time through.  The problem,
however, is that the H264Context that was passed in belongs to the
AVCodecParserContext and is uninitialized.  In particular, low_delay =
0 and that eventually leads to has_b_frames = 1.  My fix is to split
off a new ff_h264_context_init() function from ff_h264_decode_init();
this will initialize the AVCodecParserContext's H264Context and also
invoke ff_h264_decode_extradata().

The fix does not break ffmpeg's unit tests (I tried with those in
libav; the ffmpeg tests seemed broken for unrelated reasons in the
snapshot I tried) and fixes the dts/pts reported by ffprobe.

Tony

diff --git a/libavcodec/h264.c b/libavcodec/h264.c
index dbf71a7..3062723 100644
--- a/libavcodec/h264.c
+++ b/libavcodec/h264.c
@@ -868,6 +868,23 @@ av_cold int ff_h264_decode_init(AVCodecContext *avctx){
     H264Context *h= avctx->priv_data;
     MpegEncContext * const s = &h->s;

+    avctx->chroma_sample_location = AVCHROMA_LOC_LEFT;
+
+    ff_h264_context_init(avctx, h);
+
+    if(avctx->codec_id == CODEC_ID_H264){
+        if(avctx->ticks_per_frame == 1){
+            s->avctx->time_base.den *=2;
+        }
+        avctx->ticks_per_frame = 2;
+    }
+
+    return 0;
+}
+
+av_cold int ff_h264_context_init(AVCodecContext *avctx, H264Context *h){
+    MpegEncContext * const s = &h->s;
+
     MPV_decode_defaults(s);

     s->avctx = avctx;
@@ -880,9 +897,7 @@ av_cold int ff_h264_decode_init(AVCodecContext *avctx){
 //    s->decode_mb= ff_h263_decode_mb;
     s->quarter_sample = 1;
     if(!avctx->has_b_frames)
-    s->low_delay= 1;
-
-    avctx->chroma_sample_location = AVCHROMA_LOC_LEFT;
+        s->low_delay= 1;

     ff_h264_decode_init_vlc();

@@ -891,18 +906,16 @@ av_cold int ff_h264_decode_init(AVCodecContext *avctx){
     h->prev_poc_msb= 1<<16;
     h->x264_build = -1;
     ff_h264_reset_sei(h);
-    if(avctx->codec_id == CODEC_ID_H264){
-        if(avctx->ticks_per_frame == 1){
-            s->avctx->time_base.den *=2;
-        }
-        avctx->ticks_per_frame = 2;
-    }

     if(avctx->extradata_size > 0 && avctx->extradata &&
         ff_h264_decode_extradata(h))
         return -1;

     if(h->sps.bitstream_restriction_flag && s->avctx->has_b_frames <
h->sps.num_reorder_frames){
+        /*
+         * I'm a bit dubious about touching has_b_frames here, but
+         * has_b_frames usually is modified in tandem with low_delay.
+         */
         s->avctx->has_b_frames = h->sps.num_reorder_frames;
         s->low_delay = 0;
     }
diff --git a/libavcodec/h264.h b/libavcodec/h264.h
index 96720ac..caa13f6 100644
--- a/libavcodec/h264.h
+++ b/libavcodec/h264.h
@@ -681,6 +681,7 @@ void ff_h264_hl_decode_mb(H264Context *h);
 int ff_h264_frame_start(H264Context *h);
 int ff_h264_decode_extradata(H264Context *h);
 av_cold int ff_h264_decode_init(AVCodecContext *avctx);
+av_cold int ff_h264_context_init(AVCodecContext *avctx, H264Context *h);
 av_cold int ff_h264_decode_end(AVCodecContext *avctx);
 av_cold void ff_h264_decode_init_vlc(void);

diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 91fbebc..cdbf96a 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -249,10 +249,7 @@ static int h264_parse(AVCodecParserContext *s,

     if (!h->got_first) {
         h->got_first = 1;
-        if (avctx->extradata_size) {
-            h->s.avctx = avctx;
-            ff_h264_decode_extradata(h);
-        }
+        ff_h264_context_init(avctx, h);
     }

     if(s->flags & PARSER_FLAG_COMPLETE_FRAMES){
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix_mpegts_h264_pts_dts.patch
Type: application/octet-stream
Size: 2906 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110419/1fceb646/attachment.obj>