[FFmpeg-devel] [PATCH] Fix h264 decoder when nal end sequence is present

[Michael Niedermayer]

On Thu, Nov 18, 2010 at 06:24:41PM -0800, Baptiste Coudurier wrote:
> Hi
>
> $subject.  The problem happens if the last frame only contains a NAL_END  
> unit. The decoder will fail with "no frame" error and won't output the  
> last frame.
>
> Please comment. I'm not sure why the if (nalsize == 1) buf_index++ was  
> added.
>
> -- 
> Baptiste COUDURIER
> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
> FFmpeg maintainer                                  http://www.ffmpeg.org

>  h264.c |   17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 7f720299cf8fbb13ecc9bc8fc59b946200146501  h264_nal_end_fix.patch
> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> index f99f7ea..640239b 100644
> --- a/libavcodec/h264.c
> +++ b/libavcodec/h264.c
> @@ -2783,14 +2783,9 @@ static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size){
>              nalsize = 0;
>              for(i = 0; i < h->nal_length_size; i++)
>                  nalsize = (nalsize << 8) | buf[buf_index++];
> -            if(nalsize <= 1 || nalsize > buf_size - buf_index){
> +            if(!nalsize || nalsize > buf_size - buf_index){

have you considered negative nalsize, these can happen through overflow
besides this i see no problem in your patch but i also dont know why the ==1
was added.
It was added by loren:   (maybe you could ask him to make sure we arent
breaking something)
That said, once above things are resolved feel free to commit

commit 8a16e111c48b622e13d7c0bf10e909d416ec89ed
Author: lorenm <lorenm at 9553f0bf-9b14-0410-a0b8-cfaf0461ba5b>
Date:   Tue Feb 14 05:40:53 2006 +0000

    fix some crashes on negative nalsize.


    git-svn-id: svn://svn.mplayerhq.hu/ffmpeg/trunk at 5022 9553f0bf-9b14-0410-a0b8-cfaf0461ba5b

diff --git a/libavcodec/h264.c b/libavcodec/h264.c
index 4c2f0cb..a90f9fa 100644
--- a/libavcodec/h264.c
+++ b/libavcodec/h264.c
@@ -7507,6 +7507,15 @@ static int decode_nal_units(H264Context *h, uint8_t *buf, int buf_size){
         nalsize = 0;
         for(i = 0; i < h->nal_length_size; i++)
             nalsize = (nalsize << 8) | buf[buf_index++];
+        if(nalsize <= 1){
+            if(nalsize == 1){
+                buf_index++;
+                continue;
+            }else{
+                av_log(h->s.avctx, AV_LOG_ERROR, "AVC: nal size %d\n", nalsize);
+                break;
+            }
+        }
       } else {
         // start code prefix search
         for(; buf_index + 3 < buf_size; buf_index++){

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

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101120/e65c36ab/attachment.pgp>