[FFmpeg-devel] [PATCH] avcodec/h264_parser: Skip Redundant Slice Info Attached Behind Frames.

[Fu, Linjie]

> On Oct 25, 2018, at 17:53, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> 
>> On Thu, Oct 25, 2018 at 11:40 AM Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>> 
>>> On Thu, Oct 25, 2018 at 8:59 AM Linjie Fu <linjie.fu at intel.com> wrote:
>>> 
>>> Skip redundant slice info attached behind frames (PPS for eaxmple) to
>>> parse timestamp correctly and produce the segment format successfully.
>>> 
>>> When PPS info is attached behind frame data whin one PES packet,
>>> h264_find_frame_end for PPS slice returns END_NOT_FOUND，and causes the
>>> following IDR frame to returning END_NOT_FOUND too. And this leads to
>>> the failure of parsing pts and operation of segment.
>>> 
>>> Skip redundant slice info to ensure the h264_parser to find the correct
>>> frame_end of following frame, and make sure parse_packet could parse the
>>> pts and dts.
>>> 
>>> Fix the pts issue for single segment.ts and the segment format
>>> failure issue for http live stream in ticket #7207.
>>> 
>>> Signed-off-by: Linjie Fu <linjie.fu at intel.com>
>>> ---
>>> libavcodec/h264_parser.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
>>> index 5f9a9c46ef..bcc97d6260 100644
>>> --- a/libavcodec/h264_parser.c
>>> +++ b/libavcodec/h264_parser.c
>>> @@ -577,6 +577,7 @@ static int h264_parse(AVCodecParserContext *s,
>>>     H264ParseContext *p = s->priv_data;
>>>     ParseContext *pc = &p->pc;
>>>     int next;
>>> +    int input_bufsize = buf_size;
>>> 
>>>     if (!p->got_first) {
>>>         p->got_first = 1;
>>> @@ -644,7 +645,10 @@ static int h264_parse(AVCodecParserContext *s,
>>> 
>>>     *poutbuf      = buf;
>>>     *poutbuf_size = buf_size;
>>> -    return next;
>>> +    if (next > 0 && next <input_bufsize)
>>> +        return input_bufsize;
>>> +    else
>>> +        return next;
>>> }
>>> 
>>> static int h264_split(AVCodecContext *avctx,
>> 
>> This doesn't seem right to me. The frames should be split in such a
>> way that the PPS ends up in front of the next frame, and not just
>> skipped at the end of the last one.
>> Instead of working around the problem of the next IDR not being read
>> properly, why not fix that problem?
>> 
> 
Thanks for your review. 

In this case, the skipped PPS is duplicated and useless. The next IDR frame has its own SPS and PPS and could be read properly after applying this patch, so the problem is fixed.

> Since the original issue is about timestamps, this reminded me of this
> patch that I've had sitting around for a while:
> https://pastebin.com/ufE6nkWr
> 
> It was originally designed to help retrieve proper timestamps when
> PPS/SPS are badly sligned to wrong PES  units, maybe it also helps
> with your case.
> However it was never included in the main codebase because it seemed
> to also cause some other issues that I didnt have the time back then
> to investigate.
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Actually, what I wonder is whether the PPS info attached at the end of a PES packet is useless in all situations. If the PPS is useful and but is only in wrong PES unit, it’s necessary to split the info.

Thanks again and will try your patch.