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

[Tony Strauss]

Reposting, as I had the the ffmpeg-devel e-mail address wrong (was using the
old one)

On Tue, Apr 19, 2011 at 11:46 PM, Tony Strauss <tony at animoto.com> wrote:

> 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){